1308994 - encodeURIComponent for sync batch/atomic upload query parameter

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P1)

Description

[Thom Chiovoloni [:tcsc] (ex-moco)]

This is extremely embarrassing, I forgot to push this after getting it as part of the required fixes in bug 1253051 comment 33. Without this, batch uploads are probably completely broken in practice :(.

I've also found another bug in some config parsing edge cases that AFAIK can't be hit at the moment, but are worth fixing nonetheless -- since if these conditions were ever hit, batching would be totally broken. LMK if I should split this off.

Note that this will probably need uplifting to wherever atomic uploads have made it by now.

Comment 1

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1308994 - Use encodeURIComponent on sync upload batch id, and fix configuration checking

Review commit: https://reviewboard.mozilla.org/r/84640/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/84640/

Comment 2

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

Comment on attachment 8799474 [details]
Bug 1308994 - Use encodeURIComponent on sync upload batch id, and fix configuration checking

https://reviewboard.mozilla.org/r/84640/#review83278

Comment 3

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

This will need uplift to 51

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b0b027c18a6
Use encodeURIComponent on sync upload batch id, and fix configuration checking r=markh

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4b0b027c18a6

Comment 6

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8799474 [details]
Bug 1308994 - Use encodeURIComponent on sync upload batch id, and fix configuration checking

Approval Request Comment
[Feature/regressing bug #]: Bug 1253051
[User impact if declined]: Can't fully deploy server-side batch API (which helps prevent sync data corruption) to production without this.
[Describe test coverage new/current, TreeHerder]: Tested manually -- LMK if I need to add tests in order for this to get uplifted.
[Risks and why]: Low risk limited to Sync. (Batch API can also be disabled on servers that have it, and most production servers don't have it yet)
[String/UUID change made/needed]: None

Comment 7

[Gerry Chang [:gchang]]

Comment on attachment 8799474 [details]
Bug 1308994 - Use encodeURIComponent on sync upload batch id, and fix configuration checking

Fix issue related to atomic upload. Take it in 51 aurora.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/589d7c55859b