Closed Bug 417218 Opened 15 years ago Closed 14 years ago

nsStandardURL::SetPath does the wrong thing when the path is '/' and is being set to ''

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta4

People

(Reporter: Waldo, Assigned: Waldo)

References

Details

Attachments

(1 file, 1 obsolete file)

random-three-o-eight:~/moz/builds/clean/dist/bin jwalden$ ./run-mozilla.sh ./xpcshell 
js> const URL = Components.Constructor("@mozilla.org/network/standard-url;1",
                                       "nsIStandardURL", "init");
js> const nsIStandardURL = Components.interfaces.nsIStandardURL;
js> var provided = new URL(nsIStandardURL.URLTYPE_AUTHORITY, 80,
                           "http://example.com", "UTF-8", null);
js> var target = new URL(nsIStandardURL.URLTYPE_AUTHORITY, 80,
"http://example.com/tests/dom/tests/mochitest/whatwg/postMessage_origin_helper.xhtml",
"UTF-8", null);
js> provided.QueryInterface(Components.interfaces.nsIURI)
[xpconnect wrapped (nsISupports, nsIStandardURL, nsIURI) @ 0x61f7a0 (native @ 0x61f504)]
js> target.QueryInterface(Components.interfaces.nsIURI)
[xpconnect wrapped (nsISupports, nsIStandardURL, nsIURI) @ 0x61fa20 (native @ 0x61f974)]
js> provided.spec
http://example.com/
js> target.spec
http://example.com/tests/dom/tests/mochitest/whatwg/postMessage_origin_helper.xhtml
js> provided.userPass = "", provided.path = ""

js> target.userPass = "", target.path = ""

js> target.spec
http://example.com/
js> provided.spec
http://example.com/
js> target.equals(provided)
false


Debugging says |SegmentIs(mBasename, other->mSpec.get(), other->mBasename)| (only mBasename, nothing else in Equals is broken) is false because mBasename differs between the two (where |this == target| and |other == provided|):

(gdb) p mBasename
$36 = {
  mPos = 52, 
  mLen = -1
}
(gdb) p other->mBasename
$37 = {
  mPos = 19, 
  mLen = 0
}


The lengths compare unequal, and SegmentIs returns PR_FALSE.

This breaks at least two of the tests I've written for my current implementation of the optional origin argument to HTML5's postMessage.  Yay for tests; boo for our URL classes.
It should be fairly obvious what's happening; we're passing an empty string, and the logic that runs for a new, empty path only runs if the current path is more than one character long -- which is an easy enough optimization for the Cut, but doing so avoids the other, necessary side effects in that case.
Attachment #303194 - Flags: superreview?(cbiesinger)
Attachment #303194 - Flags: review?(cbiesinger)
Attachment #303194 - Attachment is obsolete: true
Attachment #303198 - Flags: superreview?(cbiesinger)
Attachment #303198 - Flags: review?(cbiesinger)
Attachment #303194 - Flags: superreview?(cbiesinger)
Attachment #303194 - Flags: review?(cbiesinger)
Attachment #303198 - Flags: superreview?(cbiesinger)
Attachment #303198 - Flags: superreview+
Attachment #303198 - Flags: review?(cbiesinger)
Attachment #303198 - Flags: review+
Comment on attachment 303198 [details] [diff] [review]
With a few more tests of other edge cases

Necessary to fix bug 417075...
Attachment #303198 - Flags: approval1.9?
Comment on attachment 303198 [details] [diff] [review]
With a few more tests of other edge cases

a=beltzner for 1.9
Attachment #303198 - Flags: approval1.9? → approval1.9+
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Summary: One of nsStandardURL's SetUserPass or SetPath function is broken → nsStandardURL::SetPath does the wrong thing when the path is '/' and is being set to ''
Target Milestone: --- → mozilla1.9beta4
You need to log in before you can comment on or make changes to this bug.