Closed Bug 81709 Opened 24 years ago Closed 23 years ago

nsStdURL should not just use CID compares in Equal().

Categories

(Core :: Networking, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED INVALID
mozilla0.9.2

People

(Reporter: dougt, Assigned: dougt)

References

Details

(Whiteboard: simple patch ready - then will be invalid)

Attachments

(2 files)

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.
Target Milestone: --- → mozilla0.9.2
mass move, v2. qa to me.
QA Contact: tever → benc
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?
IMO the I/O service should have this mapping... and it should utilize a GetDefaultPortForScheme()-type method on nsIProtocolHandler.
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?
never mind that nsSocketTransport patch...
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.
sr=darin on the second patch.
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.
Whiteboard: simple patch ready - then will be invalid
Keywords: approval
Blocks: 83989
a= asa@mozilla.org for checkin to the trunk. (on behalf of drivers)
Thanks. the patch was checked in. Now, I can mark this bug invalid.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → INVALID
(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.

Attachment

General

Created:
Updated:
Size: