Closed
Bug 81709
Opened 24 years ago
Closed 23 years ago
nsStdURL should not just use CID compares in Equal().
Categories
(Core :: Networking, defect)
Tracking
()
RESOLVED
INVALID
mozilla0.9.2
People
(Reporter: dougt, Assigned: dougt)
References
Details
(Whiteboard: simple patch ready - then will be invalid)
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
To preserve pluggable protocols, our nsStdURL should do more than just compare
against its own class. If the CID check files, nsStdURL should check each field
for equality.
Assignee | ||
Updated•24 years ago
|
Target Milestone: --- → mozilla0.9.2
Assignee | ||
Comment 2•23 years ago
|
||
I have run into a snag while hacking on a patch for this bug. Basically what I
was going to do is when the QI fails for the implement CID, I would compare
specs like:
nsXPIDLCString ourSpec, theirSpec;
rv = GetSpec(getter_Copies(ourSpec));
if (NS_FAILED(rv))
return rv;
rv = i_OtherURI->GetSpec(getter_Copies(theirSpec));
if (NS_FAILED(rv))
return rv;
*o_Equals = PL_strcasecmp(ourSpec, theirSpec) ? PR_FALSE : PR_TRUE;
The problems that I am faced with is that this will fail on implict vs.
explicted default ports. For example two url spec's that should be the same:
http://foo.com:80
http://foo.com
will not evaluate to be the same thing!
This problem forces me to do two things - first I need to do field by field
compar so that I can special case the port number. Not a problem, just more
code....
The second problem is that I need to know about the mapping between protocol
scheme and default port.
The protocol handler still has a defaultPort attribute, but darin and I agreed
that this probably should go away (since handlers can support multiple
protocols). Where should this mapping exists? Ideas?
Comment 3•23 years ago
|
||
IMO the I/O service should have this mapping... and it should utilize a
GetDefaultPortForScheme()-type method on nsIProtocolHandler.
Assignee | ||
Comment 4•23 years ago
|
||
okay, so -
Add a new function to nsIProtocolHandler. Something like:
void GetDefaultPortForScheme(in string scheme, out long port);
Add a new function to the IOService. Something like:
void GetDefaultPortForScheme(in string scheme, out long port);
The IOService implementation merely calls GetDefaultPortForScheme() on the
handler registered for the |in| scheme.
Sounds okay with me. Jud, any concerns?
Assignee | ||
Comment 5•23 years ago
|
||
Assignee | ||
Comment 6•23 years ago
|
||
never mind that nsSocketTransport patch...
Assignee | ||
Comment 7•23 years ago
|
||
Assignee | ||
Comment 8•23 years ago
|
||
the last patch is for correctness. I would like to check it in. rick/darin,
please review/sr.
However, after talking to rpotts, we agreed that it should be impossible for two
nsIURI to equal if they are implemented by different classes. The reason for
this is that we assume that for each protocol scheme there exists only one uri
implemention.
There are some serious problems with mozilla's uri parsing/nsStdURL API story if
we have to bandaide the problem as the first patch suggests. Rick and I will
meet later this week to hash out the details of the uri parsing story. I will
post something to the netlib ng next week.
Comment 9•23 years ago
|
||
sr=darin on the second patch.
Comment 10•23 years ago
|
||
r=rpotts for the second patch...
Doug and I had a long talk and agreed that in the current Necko model, there
should only be *one* nsIURI implementation per protocol - therefore, if a
protocol uses nsStdUrl as its URL implementation, then the second nsIURI must
also be a nsStdUrl before they can possibly be equal...
Currently, there are URI parsing bugs such as bug #73845 which allow nsStdUrl to
masquarade as some random protocol - but once these bugs have been fixed
nsStdUrl::Equals(...) will work as advertised :-)
I think that it is important that we inforce our assumption that there is a
single URI implementation per protocol.
Assignee | ||
Updated•23 years ago
|
Whiteboard: simple patch ready - then will be invalid
Comment 11•23 years ago
|
||
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Assignee | ||
Comment 12•23 years ago
|
||
Thanks. the patch was checked in. Now, I can mark this bug invalid.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → INVALID
Comment 13•22 years ago
|
||
(not sure how a the patch can be checked in, making this bug invalid...)
You need to log in
before you can comment on or make changes to this bug.
Description
•