Closed
Bug 568587
Opened 14 years ago
Closed 14 years ago
Record ids with / are not accepted
Categories
(Cloud Services Graveyard :: Server: Sync, defect)
Cloud Services Graveyard
Server: Sync
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Mardak, Assigned: tarek)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file, 2 obsolete files)
3.06 KB,
patch
|
telliott
:
review+
|
Details | Diff | Splinter Review |
The form rewrite generates GUIDs with btoa and base64 uses alphanumeric, +, and /. Technically there's nothing really preventing / from being in a guid, but it might be confusing in the context of a url, but that can always be escaped correctly. http://hg.mozilla.org/mozilla-central/rev/cabbe056fd9c#l5.453 2010-05-27 11:18:02Collection TRACEPOST body: {"success":["z_SoDELu5o","!wh(i5hBcv","oONRKANrAR","Ig3*CSQ0J(","3xLzv3azRH","r_LTxCdtHa","Hm.wVChXO*","Bnn(UHwAhx","_nRmhe)k1t"],"failed":{"tXue2\/2jokyBLT\/f":["invalid id"],"Vf\/7xRHJ1E+B1hnS":["invalid id"],"z\/QGd990sEOMFn9P":["invalid id"],"0\/Onl9fnCUKpVfrF":["invalid id"]}} Seems like satchel is generating 16 character guids... so 63/64 probability that a character won't be a / out of 16 characters.. 22.27% chance that the guid will contain a /?
Comment 1•14 years ago
|
||
Err, I don't believe the JS rewrite changed anything here. The C++ code (as of bug 506402) was using PL_Base64Encode() to generate 16 character GUIDs. That bug also landed tests, and those same tests continue to be run successfully, unmodified, with the JS rewrite.
Reporter | ||
Comment 2•14 years ago
|
||
Oh indeed. I grabbed a random build 20100520 and generated some guids and indeed there was a /.
Assignee | ||
Comment 3•14 years ago
|
||
The attached patch enables slashes in ids. Note that we will need to activate AllowEncodedSlashes in Apache for this to work.
Attachment #471459 -
Flags: review?(telliott)
Assignee | ||
Comment 4•14 years ago
|
||
Use php-style indentation
Attachment #471459 -
Attachment is obsolete: true
Attachment #471489 -
Flags: review?(telliott)
Attachment #471459 -
Flags: review?(telliott)
Updated•14 years ago
|
Assignee: telliott → tarek
Comment 5•14 years ago
|
||
Comment on attachment 471489 [details] [diff] [review] Allows slashes in ids rather than doing such an extensive regex, wouldn't it be more efficient to do $ipath = explode('/', $path . '///'); $username = shift($ipath); $function = shift($ipath); if ($ipath) $id = implode('/', $ipath) (syntax hasn't been checked for exactness) ?
Assignee | ||
Comment 6•14 years ago
|
||
OK - Python usually uses compiled regexp, but it sounds more efficient this way in PHP. So I suppose 'shift' is 'array_shift' here, here's a new patch using a regular split.
Attachment #471489 -
Attachment is obsolete: true
Attachment #475495 -
Flags: review?(telliott)
Attachment #471489 -
Flags: review?(telliott)
Comment 7•14 years ago
|
||
Let's get this wrapped up and on the 1.3.x branch? This prevents some form history records from syncing.
Updated•14 years ago
|
Attachment #475495 -
Flags: review?(telliott) → review+
Assignee | ||
Comment 8•14 years ago
|
||
Fixed in all branches: - stable: http://hg.mozilla.org/services/sync-server/rev/9f1f11605f8d - default: http://hg.mozilla.org/services/sync-server/rev/92e488eaaff1 Toby, looks like 1.0/tests/test_weave_utils.php was not commited
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Whiteboard: [qa-]
Updated•1 year ago
|
Product: Cloud Services → Cloud Services Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•