Closed
Bug 1274044
Opened 9 years ago
Closed 9 years ago
"ASSERTION: Caller should check its passed-in value and pass -1 instead of mDefaultPort..."
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: jruderman, Assigned: dholbert)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, regression, testcase, Whiteboard: [necko-active])
Attachments
(3 files)
###!!! ASSERTION: Caller should check its passed-in value and pass -1 instead of mDefaultPort, to avoid encoding default port into mSpec: 'aNewPort != mDefaultPort', file netwerk/base/nsStandardURL.cpp, line 1862
This assertion was added in:
changeset: https://hg.mozilla.org/mozilla-central/rev/66ff85f4450f
user: Daniel Holbert
date: Wed Feb 17 19:24:34 2016 -0800
summary: Bug 1247733 part 1: Create a helper function for nsStandardURL's code to add/remove/replace a port in the URL string. r=valentin
However, I suspect this bug is more recent.
Reporter | ||
Comment 1•9 years ago
|
||
Comment 2•9 years ago
|
||
URL code - Valentin?
Assignee: nobody → valentin.gosu
Whiteboard: [necko-active]
Assignee | ||
Comment 3•9 years ago
|
||
So the "problem" here is that mDefaultPort is *itself* -1.
The caller (SetPort) is doing what the assertion asks -- it checks if the new port matches the default port -- and it does -- and so it passes -1 instead. But then it turns out they're all -1, so the assertion complains because it thinks the caller is just passing in the default port without having checked.
The assertion needs a tweak to allow for aNewPort to match mDefaultPort *if* they're both -1.
Assignee | ||
Updated•9 years ago
|
Assignee: valentin.gosu → dholbert
Assignee | ||
Comment 4•9 years ago
|
||
(note that mDefaultPort gets initialized to -1 in the constructor, so it's not too surprising that it happens to have a value of -1 here.)
Assignee | ||
Comment 5•9 years ago
|
||
The assertion is trying to require that the caller should never pass in mDefaultPort; instead, the caller should pass -1. BUT, if mDefaultPort is itself -1 (i.e. there's no default port), then we need to allow the caller to pass in -1 without spamming an assertion.
Review commit: https://reviewboard.mozilla.org/r/53980/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53980/
Attachment #8754463 -
Flags: review?(valentin.gosu)
Comment 6•9 years ago
|
||
Comment on attachment 8754463 [details]
MozReview Request: Bug 1274044: Relax assertion in nsStandardURL for case when mDefaultPort is -1. r?valentin
https://reviewboard.mozilla.org/r/53980/#review50700
Attachment #8754463 -
Flags: review?(valentin.gosu) → review+
Comment 8•9 years ago
|
||
dholbert - Does this affect 48 and if so should it be uplifted?
tracking-firefox49:
--- → +
Flags: needinfo?(dholbert)
Assignee | ||
Comment 9•9 years ago
|
||
I think it affects every release that has bug 1247733's fix. However, there's no need to uplift; the bug here is simply that this (debug-only) assertion was a bit too strict, and the fix relaxes the assertion a bit.
The fix has no impact on opt build behavior.
Flags: needinfo?(dholbert)
Comment 10•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•