Closed Bug 1274044 Opened 5 years ago Closed 5 years ago
"ASSERTION: Caller should check its passed-in value and pass -1 instead of m
Default Port ..."
###!!! 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.
URL code - Valentin?
Assignee: nobody → valentin.gosu
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.
(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.)
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 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+
dholbert - Does this affect 48 and if so should it be uplifted?
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.
You need to log in before you can comment on or make changes to this bug.