Closed Bug 113206 Opened 23 years ago Closed 22 years ago

fix default port selection of nsHttpHandler::NewURI

Categories

(Core :: Networking: HTTP, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.2alpha

People

(Reporter: darin.moz, Assigned: darin.moz)

References

Details

(Keywords: embed, Whiteboard: [adt2] [ETA 6/26])

Attachments

(1 file, 1 obsolete file)

see nsHttpHandler.cpp:1610

  // XXX need to choose the default port based on the scheme
  rv = url->Init(nsIStandardURL::URLTYPE_AUTHORITY, 80, aSpec, aBaseURI);

this should probably be fixed.
Severity: normal → minor
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → mozilla1.0
hasn't shown up as a critical bug yet, so punt until moz 1.1
Target Milestone: mozilla1.0 → mozilla1.1
Severity: minor → normal
Keywords: embed
Priority: P4 → P3
Target Milestone: mozilla1.1alpha → mozilla1.0.1
*** Bug 118791 has been marked as a duplicate of this bug. ***
*** Bug 140100 has been marked as a duplicate of this bug. ***
ok, there's some real world sites hitting this bug... upping the priority on
this one.
Severity: normal → major
Keywords: mozilla1.0, nsbeta1
Priority: P3 → P2
Target Milestone: mozilla1.0.1 → mozilla1.0
-> 1.0.1
Whiteboard: [RTM]
Target Milestone: mozilla1.0 → mozilla1.0.1
Target Milestone: mozilla1.0.1 → ---
Target Milestone: --- → mozilla1.0.1
Whiteboard: [RTM] → [adt1 RTM]
making nsbeta1+ per Comment #4 From Darin Fisher, so that it shows up on ADT
querries. 

Is we believe this absolutely need to go out with 1.0.1, pls add an ETA to the
Status Whitreboard.
Blocks: 143047
Keywords: nsbeta1nsbeta1+
Whiteboard: [adt1 RTM] → [adt1 RTM] [ETA Needed]
Whiteboard: [adt1 RTM] [ETA Needed] → [adt1 RTM] [ETA 6/26]
i discussed this one with mitch, and we agreed that this is probably not
critical enough for 1.0.1 (the current workarounds are fine), pushing out to
1.2alpha.

adding nsbeta1-
Keywords: nsbeta1+nsbeta1-
Target Milestone: mozilla1.0.1 → mozilla1.2alpha
cleaning up adt1 rtm bugs, downgrading this to adt2.
Whiteboard: [adt1 RTM] [ETA 6/26] → [adt2] [ETA 6/26]
Attached patch v1 patch (obsolete) — Splinter Review
Keywords: patch
Comment on attachment 94934 [details] [diff] [review]
v1 patch

it is a shame that you have to create a new wrapper class to do this.

Also, you might want to use the safe forwarders unless you are completely sure
that nsHttpHandler::get() will return something.

r=dougt
Attachment #94934 - Flags: review+
dougt: good point... i was working off the assumption that no one would own a
reference to the nsHttpsHandler after service manager shutdown, but actually
that's not a valid assumption... revising patch...
Attached patch v1.1 patchSplinter Review
revised patch to make nsHttpsHandler own a reference to the nsHttpHandler
instance to ensure that it can safely call through without checking for the
existance of the nsHttpHandler.
Attachment #94934 - Attachment is obsolete: true
Comment on attachment 94947 [details] [diff] [review]
v1.1 patch

r=dougt
Attachment #94947 - Flags: review+
Comment on attachment 94934 [details] [diff] [review]
v1 patch

sr=alecf - sorry for the delay!
Attachment #94934 - Flags: superreview+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
verified - confirmed patch checked in on branch and trunk
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: