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

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

See bug 388186 comment 13.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

I think we should fix this on branches too, since security code depends on origins, which are hostPort.  Seem reasonable?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Slightly better

This handles SetPort() to mDefaultPort more correctly, imo.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

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 -=

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: 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.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

sounds good to me.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 272596 [details] [diff] [review]
Updated to comments

approved for 1.8.1.7, a=dveditz for release-drivers

Comment 8

[Boris Zbarsky [:bzbarsky]]

Fixed on branch.