Closed
Bug 977844
Opened 11 years ago
Closed 11 years ago
Make sure snippets response is valid JSON before caching it (or trying to use it)
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox29 fixed, firefox30 fixed, fennec29+)
RESOLVED
FIXED
Firefox 30
People
(Reporter: Margaret, Assigned: Margaret)
References
Details
Attachments
(1 file)
3.68 KB,
patch
|
bnicholson
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 2•11 years ago
|
||
To test this locally, I just hacked the value of resposeText to some bogus value, and the error got reported as expected, and nothing ended up getting cached.
Attachment #8383340 -
Flags: review?(bnicholson)
Comment 3•11 years ago
|
||
Comment on attachment 8383340 [details] [diff] [review] patch Review of attachment 8383340 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/components/Snippets.js @@ +119,5 @@ > function updateSnippets() { > _httpGetRequest(gSnippetsURL, function(responseText) { > + try { > + let messages = JSON.parse(responseText); > + cacheSnippets(messages); Can you just pass in responseText directly here? At this point, you know it's valid since you already did JSON.parse(). @@ +135,3 @@ > */ > +function cacheSnippets(messages) { > + let data = gEncoder.encode(JSON.stringify(messages)); ...that way, you don't need to re-stringify the string you just parsed.
Attachment #8383340 -
Flags: review?(bnicholson) → review+
Assignee | ||
Comment 4•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6de7f6039a68
Comment 5•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6de7f6039a68
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 8383340 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): enabling snippets User impact if declined: snippets can get in busted state if server returns bad value Testing completed (on m-c, etc.): baked on nightly since 2/28 and tested locally, since we don't have any active snippets on Nightly right now Risk to taking this patch (and alternatives if risky): low-risk, makes snippet caching more robust String or IDL/UUID changes made by this patch: none
Attachment #8383340 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
status-firefox29:
--- → affected
status-firefox30:
--- → fixed
Updated•11 years ago
|
Attachment #8383340 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•