Closed Bug 1418438 Opened 7 years ago Closed 7 years ago

document.docShell.forcedCharset crashes Firefox

Categories

(Core :: XPConnect, defect)

Product:
Core
Component:
XPConnect
Version:
56 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- wontfix
firefox58 --- verified
firefox59 --- verified

People

(Reporter: Oriol, Assigned: Oriol)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

Bug 1418438 - Avoid null deref in nsIDocShell's forcedCharset.
(deleted), text/x-review-board-request
emk
: review+
gchang
: approval-mozilla-beta+
Details
Blocks: 1418365
status-firefox57: --- → affected
status-firefox58: --- → affected
status-firefox59: --- → affected
status-firefox-esr52: --- → unaffected
Version: unspecified → 56 Branch
Comment on attachment 8929593 [details] Bug 1418438 - Avoid null deref in nsIDocShell's forcedCharset. https://reviewboard.mozilla.org/r/200866/#review206184 ::: docshell/base/nsDocShell.cpp:2190 (Diff revision 1) > } > > NS_IMETHODIMP > nsDocShell::GetForcedCharset(nsACString& aResult) > { > + if (mForcedCharset) { Please truncate aCharset if mForcedCharset is null (although it is not relevant to JS callers).
Attachment #8929593 - Flags: review?(VYV03354) → review+
Comment on attachment 8929593 [details] Bug 1418438 - Avoid null deref in nsIDocShell's forcedCharset. https://reviewboard.mozilla.org/r/200866/#review206184 > Please truncate aCharset if mForcedCharset is null (although it is not relevant to JS callers). Thanks, I considered it but thought it wasn't necessary.
Assignee: nobody → oriol-bugzilla
Status: NEW → ASSIGNED
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f6a49a33c53d Avoid null deref in nsIDocShell's forcedCharset. r=emk
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f6a49a33c53d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Hi :Oriol, do you think if it's worth uplifting to 58 if this patch is not too risky?
Flags: needinfo?(oriol-bugzilla)
Comment on attachment 8929593 [details] Bug 1418438 - Avoid null deref in nsIDocShell's forcedCharset. Approval Request Comment [Feature/Bug causing the regression]: bug 1373984. [User impact if declined]: crash when accessing an empty nsIDocShell's forcedCharset. [Is this code covered by automated tests?]: no. [Has the fix been verified in Nightly?]: I have manually verified this in Nightly. [Needs manual test from QE? If yes, steps to reproduce]: see comment 0. [List of other uplifts needed for the feature/fix]: none. [Is the change risky?]: not risky. [Why is the change risky/not risky?]: just a check to avoid a null deref. [String changes made/needed]: none.
Flags: needinfo?(oriol-bugzilla)
Attachment #8929593 - Flags: approval-mozilla-beta?
Tiny volume, not taking it in 57.
status-firefox57: affected → wontfix
Comment on attachment 8929593 [details] Bug 1418438 - Avoid null deref in nsIDocShell's forcedCharset. Fix a potential crash. Beta58+.
Attachment #8929593 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Verified as fixed with 59.oa1 20171126220311 and 58.0b6 on Windows 10 x64, macOS 10.12.6 and Ubuntu 14.4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: