Closed Bug 1340695 Opened 8 years ago Closed 8 years ago

RustURL's SchemeIs should do a case insensitive string comparison

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

Details

Attachments

(1 file)

Assignee: nobody → ehsan
Attachment #8838816 - Flags: review?(mcmanus) → review+
Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/239f0df955d1 Perform a case insensitive string comparison in RustURL::SchemeIs(); r=mcmanus
The scheme of an `Url` object is always lower-cased by the parser, so if `SchemeIs` is only used with literal strings that are also lowercase, then the case-sensitivity of the comparison doesn’t change anything.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
I'm with Simon, are we sure this was a good change? Or do we have legacy callers that don't provide lowercase input?
Ehsan, was there a specific bug that motivated this change?
Flags: needinfo?(ehsan)
(In reply to Simon Sapin (:SimonSapin) from comment #3) > The scheme of an `Url` object is always lower-cased by the parser, so if > `SchemeIs` is only used with literal strings that are also lowercase, then > the case-sensitivity of the comparison doesn’t change anything. There is nothing mandating what kind of strings can be passed to SchemeIs. But yes, if you pass a lower case string to that function then this patch won't change anything. The test that landed with it should have made this quite clear. (In reply to Anne (:annevk) from comment #5) > I'm with Simon, are we sure this was a good change? Yes. Any implementation of nsIURI needs to adhere to the same API contract. If we think that an API contract needs to be changed, such a change needs to be made to all nsIURI implementations. RustURL doesn't get to implement its own semantics. (In reply to Simon Sapin (:SimonSapin) from comment #6) > Ehsan, was there a specific bug that motivated this change? No, I found this during an audit of the code looking for an unrelated issue.
Flags: needinfo?(ehsan)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: