Closed Bug 600995 Opened 12 years ago Closed 12 years ago

Relative URI handling confused by slashes in form records

Categories

(Firefox :: Sync, defect)

x86
Linux
defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: jbecerra, Assigned: philikon)

References

(Depends on 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20100930 Firefox/4.0b7pre

I was installing a new Linux VM, and I wanted to try Sync using my personal account. I looked in about:sync-log out of curiosity and saw lots of errors like the one in the summary.

See about:sync-log and about:sync-log.1 attachments.

The information was originally sync'd using Namoroka with Sync 1.5b6
Attached file about sync-log
Attached file about sync-log.1
Excluding bookmark synchronization part
Assignee: nobody → philipp
Comment on attachment 479939 [details]
about sync-log.1

>.....
>2010-09-30 14:41:15	Collection           DEBUG	GET success 200 https://phx-sync074.services.mozilla.com/1.0/juanb/storage/bookmarks?full=1
>2010-09-30 14:41:15	Engine.Bookmarks     INFO	Records: 3488 applied, 0 reconciled, 0 left to fetch
>2010-09-30 14:41:15	Engine.Bookmarks     INFO	Uploading all of 13 records
>2010-09-30 14:41:15	Collection           DEBUG	POST Length: 8995
>2010-09-30 14:41:16	Collection           DEBUG	POST success 200 https://phx-sync074.services.mozilla.com/1.0/juanb/storage/bookmarks
>2010-09-30 14:41:16	Collection           DEBUG	DELETE success 200 https://phx-sync074.services.mozilla.com/1.0/juanb/storage/bookmarks?ids={ed2c27f7-ea41-41ea-a9d6-bf951235da7b}1,{ed2c27f7-ea41-41ea-a9d6-bf951235da7b}11,{ed2c27f7-ea41-41ea-a9d6-bfâÃâ¬Ã¦
>2010-09-30 14:41:17	Engine.Bookmarks     DEBUG	Total (ms): sync 118388, processIncoming 116823, uploadOutgoing 1083, syncStartup 135, syncFinish 31, createRecord 32, findDupe 106, handleDupe 23, resetLastSync 0, deleteId 0, reconcile 3870, resetClient 0

I'm guessing bookmarks and the other engines synced without these error messages?

>2010-09-30 14:41:17	Net.Resource         DEBUG	GET success 200 https://phx-sync074.services.mozilla.com/1.0/juanb/storage/crypto/forms
>2010-09-30 14:41:17	Engine.Forms         DEBUG	Engine syncIDs: a-tY4We0Ax,iijrvFEi62
>2010-09-30 14:41:17	Engine.Forms         DEBUG	Resetting forms last sync time
>2010-09-30 14:41:17	Engine.Forms         DEBUG	First sync, uploading all items
>2010-09-30 14:41:17	Engine.Forms         INFO	0 outgoing items pre-reconciliation
>2010-09-30 14:41:18	Net.Resource         DEBUG	GET fail 400 https://phx-sync074.services.mozilla.com/1.0/juanb/crypto/forms
>2010-09-30 14:41:18	Engine.Forms         WARN	Error processing record: meta is null JS Stack trace: CryptoWrapper_decrypt([object Object])@crypto.js:84 < ([object Object])@engines.js:471 < innerBind([object Object])@util.js:904 < ()@collection.js:160 < Channel_onDataAvail([object XPCWrappedNative_NoHelper],null,[object XPCWrappedNative_NoHelper],3839,111472)@resource.js:372

It's missing the "storage/" part of the URL here. This sounds like a problem with the relative URL stuff. It only seems to happen for 56 of your 770 form records. My guess is that these are form records that have slashes in their GUIDs, which utterly confuses our relative URI handling. It created the encryption record as "../../crypto/forms" (whereas "../crypto/forms" would've been correct) which then got applied to the stub URI we provide for new records, eating the additional "storage/" part of the URI.
I believe we should do two things here:

1. Ignore whatever 'encryption' property we find on the record. They might be wrong as we've seen here, and the engine knows it already anyway. So let's just short-circuit that to engine's value.

2. Generate proper relative URIs and record IDs. Slashes in record IDs feels like asking for disaster, so perhaps the form engine should just translate slashes to something safer.
Summary: null JS Stack trace: CryptoWrapper_decrypt([object Object])@crypto.js:84 < ([object Object])@engines.js:471 < innerBind([object Object])@util.js:904 < ()@collection.js:160 < Channel_onDataAvail([object XPCWrappedNative_NoHelper],null [object XPCWrappedNat → Relative URI handling confused by slashes in form records
Here's a patch for 1)
Attachment #480198 - Flags: review?(mconnor)
Implement fallback that actually makes sense: Try to decrypt with engine.cryptoMetaURL, if that fails, use item.encryption.
Attachment #480198 - Attachment is obsolete: true
Attachment #480595 - Flags: review?(mconnor)
Attachment #480198 - Flags: review?(mconnor)
Comment on attachment 480595 [details] [diff] [review]
Part 1 (v2): short-circuit record.encryption

this is better, please note the bug # in the comment about relative URI confusions.
Attachment #480595 - Flags: review?(mconnor) → review+
Landed Part 1: http://hg.mozilla.org/services/fx-sync/rev/57142f068ebd

We probably want to fix part 2 in satchel itself. Filed bug 601730 for that. Resolving this one.
Status: NEW → RESOLVED
Closed: 12 years ago
Depends on: 601730
Resolution: --- → FIXED
verified with 1.5b7 on nightly Minefield builds
Status: RESOLVED → VERIFIED
This only seems to be partially fixed.  Form records with slashes in them can still get stuck by syncing client B before 1.5b7 is installed.  The only way I found to make this work right is to not sync on each client 'til 1.5b7 is installed.

scenario with bug:
1) client A with 1.5b7, sync it 
2) client B without 1.5b7, sync it. observe bug in slash containing form items
3) upgrade client B with 1.5b7, sync it. bug remains

scenario without bug:
1) client A with 1.5b7, sync it 
2) fresh profile on client B with 1.5b7 installed before ever syncing, sync it.  no bug, slashed form items sync fine.

should this be reopened? or is the first scenario unavoidable?
Blocks: 601952
Depends on: 601973
Just tested this against Fennec 4.0 beta 1 build 2, and i'm not seeing the fix there.  

about:sync-log shows:
Net.Resource     DEBUG   fail 400 https://phx-sync215.services.mozilla.com/1.0/<myDeviceName>/crypto/forms
Engine.Forms     WARN    Error processing record: meta is null JS Stack trace: CrytpWrapper_decrypt([object Object])@crypto.js:84

PK says it should be fixed in today's nightly though.  will try this again to verify.
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.