Closed Bug 1261318 Opened 4 years ago Closed 4 years ago

crash in mozilla::net::nsHTTPCompressConv::OnStopRequest

Categories

(Core :: Networking: HTTP, defect)

45 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox45 --- affected
firefox46 --- fixed
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: philipp, Assigned: mcmanus)

References

Details

(Keywords: crash, regression, Whiteboard: [necko-active])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-61c234c5-ffc4-47c1-bc90-260fd2160220.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::net::nsHTTPCompressConv::OnStopRequest(nsIRequest*, nsISupports*, nsresult) 	netwerk/streamconv/converters/nsHTTPCompressConv.cpp
1 	browsercomps.dll 	nsFeedSniffer::ConvertEncodedData(nsIRequest*, unsigned char const*, unsigned int) 	browser/components/feeds/nsFeedSniffer.cpp
2 	xul.dll 	js::SavedFrame::finishSavedFrameInit(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>) 	js/src/vm/SavedStacks.cpp
3 	xul.dll 	nsRange::AutoInvalidateSelection::~AutoInvalidateSelection() 	dom/base/nsRange.cpp
4 	browsercomps.dll 	nsFeedSniffer::GetMIMETypeFromContent(nsIRequest*, unsigned char const*, unsigned int, nsACString&) 	browser/components/feeds/nsFeedSniffer.cpp
5 	xul.dll 	NS_SniffContent(char const*, nsIRequest*, unsigned char const*, unsigned int, nsACString_internal&) 	netwerk/base/nsNetUtil.cpp
6 	xul.dll 	mozilla::net::CallTypeSniffers 	netwerk/protocol/http/nsHttpChannel.cpp
7 	xul.dll 	CallPeekFunc 	netwerk/base/nsInputStreamPump.cpp
8 	xul.dll 	mozilla::net::CacheFileInputStream::ReadSegments(nsresult (*)(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) 	netwerk/cache2/CacheFileInputStream.cpp
9 	xul.dll 	nsInputStreamPump::PeekStream(void (*)(void*, unsigned char const*, unsigned int), void*) 	netwerk/base/nsInputStreamPump.cpp
10 	xul.dll 	CallPeekFunc 	netwerk/base/nsInputStreamPump.cpp
11 	xul.dll 	mozilla::net::nsHttpChannel::ContinueOnStartRequest2(nsresult) 	netwerk/protocol/http/nsHttpChannel.cpp

this signature is first regressing in 44 and seems to be related to brotli compression that landed with bug 366559.
Assignee: nobody → mcmanus
Whiteboard: [necko-active]
a fairly big brotli change landed in 46 and this signature changed:

https://crash-stats.mozilla.com/report/index/71446acd-54e1-49a1-ae3b-9f04c2160322
https://crash-stats.mozilla.com/report/index/0df6a8e5-dfb0-4133-a1c9-a11822160326
https://crash-stats.mozilla.com/report/index/fab649db-7a1b-4a6d-aff4-2e2182160328
https://crash-stats.mozilla.com/report/index/fc1222ec-333c-45f0-8e2b-900472160330
https://crash-stats.mozilla.com/report/index/1a423587-2f51-49e5-bc1f-6244b2160327

the source of that is pretty clear - mBrotli is lazily created in the first ondataavailable.. so we just need to null check it.

Its not clear to me if this is just covering up the original signature or not - the change in 46 is too big to really ascertain.
Attachment #8737310 - Flags: review?(daniel) → review+
https://hg.mozilla.org/mozilla-central/rev/ce36169cf980
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment on attachment 8737310 [details] [diff] [review]
make sure brotli context is created in onstoprequest

crash fix

Approval Request Comment
[Feature/regressing bug #]: brotli support in 44
[User impact if declined]: corner case null deref (when connection is lost after declaring brotli but before sending any encoded data)
[Describe test coverage new/current, TreeHerder]: regression coverage is ok - nothing new
[Risks and why]: very low - literally a null check
[String/UUID change made/needed]:
Attachment #8737310 - Flags: approval-mozilla-beta?
Attachment #8737310 - Flags: approval-mozilla-aurora?
Comment on attachment 8737310 [details] [diff] [review]
make sure brotli context is created in onstoprequest

Crash fix, affects beta, ok to uplift to aurora and beta.
Attachment #8737310 - Flags: approval-mozilla-beta?
Attachment #8737310 - Flags: approval-mozilla-beta+
Attachment #8737310 - Flags: approval-mozilla-aurora?
Attachment #8737310 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.