Last Comment Bug 388281 - [FIX]nsStandardURL::SetPort needs to update authority length
: [FIX]nsStandardURL::SetPort needs to update authority length
Status: RESOLVED FIXED
: fixed1.8.1.8
Product: Core
Classification: Components
Component: Networking (show other bugs)
: unspecified
: All All
: P1 normal (vote)
: mozilla1.9alpha8
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-16 00:36 PDT by Boris Zbarsky [:bz]
Modified: 2007-08-30 08:48 PDT (History)
3 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (2.59 KB, patch)
2007-07-16 00:55 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Review
Slightly better (2.76 KB, patch)
2007-07-16 00:59 PDT, Boris Zbarsky [:bz]
cbiesinger: review+
cbiesinger: superreview+
Details | Diff | Review
Updated to comments (2.88 KB, patch)
2007-07-16 21:50 PDT, Boris Zbarsky [:bz]
dveditz: approval1.8.1.8+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2007-07-16 00:36:53 PDT
See bug 388186 comment 13.
Comment 1 Boris Zbarsky [:bz] 2007-07-16 00:55:05 PDT
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?
Comment 2 Boris Zbarsky [:bz] 2007-07-16 00:59:04 PDT
Created attachment 272460 [details] [diff] [review]
Slightly better

This handles SetPort() to mDefaultPort more correctly, imo.
Comment 3 Christian :Biesinger (don't email me, ping me on IRC) 2007-07-16 17:01:24 PDT
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 [:bz] 2007-07-16 21:50:07 PDT
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.
Comment 5 Christian :Biesinger (don't email me, ping me on IRC) 2007-07-16 22:05:10 PDT
sounds good to me.
Comment 6 Boris Zbarsky [:bz] 2007-07-16 22:24:44 PDT
Checked in.
Comment 7 Daniel Veditz [:dveditz] 2007-08-29 15:42:21 PDT
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 [:bz] 2007-08-30 08:48:53 PDT
Fixed on branch.

Note You need to log in before you can comment on or make changes to this bug.