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)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file)
(deleted),
patch
|
mcmanus
:
review+
|
Details | Diff | Splinter Review |
It currently performs a case sensitive one: <http://searchfox.org/mozilla-central/rev/59cd73fbfa14384a81a4e3eb17d3881b372702c4/netwerk/base/RustURL.cpp#318>.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ehsan
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8838816 -
Flags: review?(mcmanus)
Updated•8 years ago
|
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
Comment 3•8 years ago
|
||
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.
Comment 4•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 5•8 years ago
|
||
I'm with Simon, are we sure this was a good change? Or do we have legacy callers that don't provide lowercase input?
Comment 6•8 years ago
|
||
Ehsan, was there a specific bug that motivated this change?
Flags: needinfo?(ehsan)
Assignee | ||
Comment 7•8 years ago
|
||
(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.
Description
•