[FIX]nsStandardURL::SetPort needs to update authority length

RESOLVED FIXED in mozilla1.9alpha8

Status

()

Core
Networking
P1
normal
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

({fixed1.8.1.8})

unspecified
mozilla1.9alpha8
fixed1.8.1.8
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

See bug 388186 comment 13.
Flags: in-testsuite?
(Assignee)

Updated

10 years ago
Priority: -- → P1
Summary: nsStandardURL::SetPort needs to update authority length → [FIX]nsStandardURL::SetPort needs to update authority length
Target Milestone: --- → mozilla1.9beta1
Created attachment 272459 [details] [diff] [review]
Fix

I think we should fix this on branches too, since security code depends on origins, which are hostPort.  Seem reasonable?
Attachment #272459 - Flags: superreview?(cbiesinger)
Attachment #272459 - Flags: review?(cbiesinger)
Created attachment 272460 [details] [diff] [review]
Slightly better

This handles SetPort() to mDefaultPort more correctly, imo.
Attachment #272459 - Attachment is obsolete: true
Attachment #272460 - Flags: superreview?(cbiesinger)
Attachment #272460 - Flags: review?(cbiesinger)
Attachment #272459 - Flags: superreview?(cbiesinger)
Attachment #272459 - Flags: review?(cbiesinger)
Comment on attachment 272460 [details] [diff] [review]
Slightly better

+        mAuthority.mLen += start - mPath.mPos;

I think it would be clearer if you wrote this as:
  mAuthority.mLen -= mPath.mPos - start;

perhaps store that length in a temp. var even and pass that to Cut and -=
Attachment #272460 - Flags: superreview?(cbiesinger)
Attachment #272460 - Flags: superreview+
Attachment #272460 - Flags: review?(cbiesinger)
Attachment #272460 - Flags: review+
Created attachment 272596 [details] [diff] [review]
Updated to comments

I made that code read:

        // need to remove the port number from the URL spec
        PRUint32 start = mHost.mPos + mHost.mLen;
        PRUint32 lengthToCut = mPath.mPos - start;
        mSpec.Cut(start, lengthToCut);
        mAuthority.mLen -= lengthToCut;
        ShiftFromPath(-lengthToCut);

I think we should take this on branch; right now if a principal's URI gets SetPort called on it somewhere along the way, the principals origin will actually be wrong.
Attachment #272460 - Attachment is obsolete: true
Attachment #272596 - Flags: approval1.8.1.6?
Attachment #272596 - Flags: approval1.8.0.13?
sounds good to me.
OS: Linux → All
Hardware: PC → All
Checked in.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Attachment #272596 - Flags: approval1.8.0.13? → approval1.8.0.14?
Comment on attachment 272596 [details] [diff] [review]
Updated to comments

approved for 1.8.1.7, a=dveditz for release-drivers
Attachment #272596 - Flags: approval1.8.1.7?
Attachment #272596 - Flags: approval1.8.1.7+
Attachment #272596 - Flags: approval1.8.0.14?
Fixed on branch.
Keywords: fixed1.8.1.7
You need to log in before you can comment on or make changes to this bug.