Closed
Bug 977844
Opened 9 years ago
Closed 9 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•9 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•9 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•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6de7f6039a68
Comment 5•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6de7f6039a68
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Assignee | ||
Comment 6•9 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•9 years ago
|
status-firefox29:
--- → affected
status-firefox30:
--- → fixed
Updated•9 years ago
|
Attachment #8383340 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•2 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
•