Closed
Bug 853823
Opened 11 years ago
Closed 11 years ago
updater should be case insensitive when parsing hash function in update xml
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 22
People
(Reporter: bhearsum, Assigned: bhearsum)
Details
Attachments
(1 file, 1 obsolete file)
621 bytes,
patch
|
kats
:
review+
bhearsum
:
checkin+
|
Details | Diff | Splinter Review |
Toolkit's nsUpdateService treats hash functions as case-insensitive (https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#3355). Firefox for Android's probably should, too. I ended up discovering this when trying to change update snippets in bug 853036 to help more easily test the new update server.
Attachment #728178 -
Flags: review?(mark.finkle)
Comment 1•11 years ago
|
||
Comment on attachment 728178 [details] [diff] [review] stop carrying about case in hash function >- if ("sha512".equals(hashFunction)) { >+ if ((hashFunction.compareToIgnoreCase("sha512")) == 0) { > javaHashFunction = "SHA-512"; > } else { > Log.e(LOGTAG, "Unhandled hash function: " + hashFunction); if (("sha512".compareToIgnoreCase(hashFunction)) == 0) { hashFunction can be null, so just use the literal string as the LHS
Attachment #728178 -
Flags: review?(mark.finkle) → review+
Comment 2•11 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #1) > if (("sha512".compareToIgnoreCase(hashFunction)) == 0) { > > hashFunction can be null, so just use the literal string as the LHS Unlike equals(), compareToIgnoreCase() throws an NPE if the argument passed in is null, so even using the literal string as the LHS doesn't solve the problem. From the docs it looks like "sha512".equalsIgnoreCase(hashFunction) should be safe but it's probably worth testing first.
Assignee | ||
Comment 3•11 years ago
|
||
Thanks for the catches. I just verified this: ➜ tmp cat java.java public class java { public static void main(String[] args) { if ("sha512".equalsIgnoreCase("SHA512")) { System.out.println("first"); } if ("sha512".equalsIgnoreCase(null)) { System.out.println("second"); } if ("sha512".compareToIgnoreCase(null) == 0) { System.out.println("third"); } } } ➜ tmp java java first Exception in thread "main" java.lang.NullPointerException at java.lang.String$CaseInsensitiveComparator.compare(String.java:1177) at java.lang.String$CaseInsensitiveComparator.compare(String.java:1170) at java.lang.String.compareToIgnoreCase(String.java:1220) at java.main(java.java:9)
Attachment #728934 -
Flags: review?(bugmail.mozilla)
Comment 4•11 years ago
|
||
Comment on attachment 728934 [details] [diff] [review] use equalsIgnoreCase Review of attachment 728934 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #728934 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Updated•11 years ago
|
Attachment #728178 -
Attachment is obsolete: true
Assignee | ||
Comment 5•11 years ago
|
||
Comment on attachment 728934 [details] [diff] [review] use equalsIgnoreCase No problem. I pushed this to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/c457bd84dc66
Attachment #728934 -
Flags: checkin+
Assignee | ||
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 6•11 years ago
|
||
Usually whoever merges the patch to m-c will mark the bug fixed.
Assignee: nobody → bhearsum
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c457bd84dc66
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Updated•3 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
•