Closed Bug 1473507 Opened 6 years ago Closed 6 years ago

Crash in nsILoadInfo::GetOriginAttributes

Categories

(Core :: DOM: Security, defect)

Unspecified
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 + fixed

People

(Reporter: calixte, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-c265d263-1cbf-4055-9bcb-1b23d0180704.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll nsILoadInfo::GetOriginAttributes dist/include/nsILoadInfo.h:441
1 xul.dll mozilla::net::HttpBaseChannel::SetReferrerWithPolicy netwerk/protocol/http/HttpBaseChannel.cpp:1679
2 xul.dll nsWebBrowserPersist::SaveURIInternal dom/webbrowserpersist/nsWebBrowserPersist.cpp:1401
3 xul.dll nsWebBrowserPersist::SavePrivacyAwareURI dom/webbrowserpersist/nsWebBrowserPersist.cpp:445
4 xul.dll XPTC__InvokebyIndex xpcom/reflect/xptcall/md/win32/xptcinvoke_asm_x86_64.asm:97
5  @0xfff9ffffffffffff 
6 xul.dll XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1186
7 xul.dll XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:893
8 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:531
9 xul.dll static bool Interpret js/src/vm/Interpreter.cpp:3195

=============================================================

There are 45 crashes (from 23 installations) in nightly 63 starting with buildid 20180704100142. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1469916.

[1] https://hg.mozilla.org/mozilla-central/rev?node=9a2b02fe351b
Flags: needinfo?(gijskruitbosch+bugs)
STR:

1. load https://upload.wikimedia.org/wikipedia/commons/3/3d/LARGE_elevation.jpg
2. click at info icon (i in circle) on the left of urlbar
3. select >
4. select more infomation
5. select media
6. save https://upload.wikimedia.org/wikipedia/commons/3/3d/LARGE_elevation.jpg

Booooooooom!!!! bp-017cf625-9fc6-4bf9-bfc8-c99ab0180705
How is it possible for a channel to have a null loadinfo (or for that to have null origin attributes)?

Also not sure how my patch triggers this...
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(ckerschb)
[Tracking Requested - why for this release]: regression, #3 top crash on the July 4 Windows Nightly.
(In reply to Ekanan Ketunuti from comment #2)
> STR:
> 
> 1. load
> https://upload.wikimedia.org/wikipedia/commons/3/3d/LARGE_elevation.jpg
> 2. click at info icon (i in circle) on the left of urlbar
> 3. select >
> 4. select more infomation
> 5. select media
> 6. save
> https://upload.wikimedia.org/wikipedia/commons/3/3d/LARGE_elevation.jpg
> 
> Booooooooom!!!! bp-017cf625-9fc6-4bf9-bfc8-c99ab0180705

Thanks, this helps. I'll just sellotape over this...

You know, you'd think that if something were called `internalSave`, people wouldn't call it willy-nilly from various end-consumers all over the codebase. But you'd be wrong...
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(ckerschb)
(also, you'd think there were automated test, and you'd be wrong about that too...)
I went over https://dxr.mozilla.org/mozilla-central/search?q=saveURL+-path%3Askia+-path%3Atest&redirect=false and https://dxr.mozilla.org/mozilla-central/search?q=internalSave(+-path%3Askia+-path%3Atest&redirect=false another time, and I don't *think* there's any other consumers that don't pass a principal or document anymore, but at least with this patch we should stop crashing in this particular spot...
Comment on attachment 8990149 [details]
Bug 1473507 - fix crash in nsILoadInfo::GetOriginAttributes when passing no principal to SavePrivacyAwareURI,

https://reviewboard.mozilla.org/r/255148/#review262046

This needs more of an explanation in the commit message. I guess the C++ change is a null check on an argument, but what is the JS change doing?
Attachment #8990149 - Flags: review?(continuation) → review-
Comment on attachment 8990149 [details]
Bug 1473507 - fix crash in nsILoadInfo::GetOriginAttributes when passing no principal to SavePrivacyAwareURI,

https://reviewboard.mozilla.org/r/255148/#review262050

Thanks.
Attachment #8990149 - Flags: review?(continuation) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ddb29b481834
fix crash in nsILoadInfo::GetOriginAttributes when passing no principal to SavePrivacyAwareURI, r=mccr8
https://hg.mozilla.org/mozilla-central/rev/ddb29b481834
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Crash Signature: [@ nsILoadInfo::GetOriginAttributes] → [@ nsILoadInfo::GetOriginAttributes] [@ mozilla::net::HttpBaseChannel::SetReferrerWithPolicy]
Depends on: 1487263
You need to log in before you can comment on or make changes to this bug.