Closed Bug 379034 Opened 17 years ago Closed 16 years ago

Relative URLs with capitailzed schemes are treated as absolute

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: brettw, Assigned: michal)

Details

Attachments

(2 files, 1 obsolete file)

If one is on an "http" page, then relative URLs can be expressed normally as "foo.html", they can also be expressed as "http:foo.html". The test for this condition is whether the schemes match. Thus, on a http page, "http:mozilla.org" tries to open the relative file "mozilla.org", but "https:mozilla.org" tries to open the absolute URL "mozilla.org".

The problem is that the test for this condition is case-sensetive, where it does a strncmp in nsStandardURL::Resolve:
http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsStandardURL.cpp#1780

This means that if I have "http:mozilla.org" it's a relative file, but if I have "Http:mozilla.org" it's an absolute URL. This is even more confusing since the latter is canonicalized to a lower case scheme, making it less clear what happened. IE does a case-insensitive compare here, and I think Mozilla should, too.
I was wrong, the call in question is the SegmentIs call right before the strncmp line mentioned above.
Attached patch fix (obsolete) — Splinter Review
Compare schemes in nsStandardURL::Resolve() ignoring case.

URLs in document http://localhost/a.html are resolved as follows

1) http:b.html -> http://localhost/b.html
2) Http:b.html -> http://localhost/b.html
3) https:b.html -> https://b.html/
4) ./https:b.html -> http://localhost/https:b.html

It is a question if case 3 shouldn't be resolved in the same way as case 4 (Opera does it, Konqueror not). But I don't think that it is an error according to section 5.3 of RFC 1808.
Assignee: nobody → michal
Status: NEW → ASSIGNED
Attachment #327943 - Flags: review?(cbiesinger)
(In reply to comment #2)
> It is a question if case 3 shouldn't be resolved in the same way as case 4
> (Opera does it, Konqueror not).

That sounds very wrong to me. I think Firefox is correct.


Off the top of my head, I think what you are saying is correct. Try to get a priority and target for this bug.
Comment on attachment 327943 [details] [diff] [review]
fix

r=biesi if you also add a unit test
Attachment #327943 - Flags: review?(cbiesinger) → review+
Attached patch unit testSplinter Review
Attachment #334295 - Flags: review?(cbiesinger)
Attachment #327943 - Flags: superreview?(bzbarsky)
Comment on attachment 327943 [details] [diff] [review]
fix

s/icase/ignoreCase/ and sr=bzbarsky
Attachment #327943 - Flags: superreview?(bzbarsky) → superreview+
Attached patch fix v2Splinter Review
Attachment #327943 - Attachment is obsolete: true
Keywords: checkin-needed
Attachment #334295 - Flags: review?(cbiesinger) → review+
Pushed to mozilla-central: https://hg.mozilla.org/mozilla-central/index.cgi/rev/b1d751307fae
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: