1310453 - Shutdown Crash in nsDataObj::GetFileContentsInternetShortcut

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect, P1)

Description

[Alice0775 White]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

This bug was filed from the Socorro interface and is 
report bp-9b0f9f5f-9dc0-412e-9f1e-f53eb2161015.
=============================================================

Shutdown Crash,

The crash is reproducible since 47 bp-50f6fac8-0282-4e64-b2af-dc4b52161015.


Reproducible: always, Also crashed in the new profile

Steps To Reproduce:
1. Open Library (Ctrl+Shift+B)
2. Select "All Bookmarks" in the left side pane
3. Select multiple folders in the right side pane
   E.g, (Bookmarks Toolbar, Bookmarks Menu and Unsorted Bookmarks)
        (note: The crash will not be limited to these special folder)
4. Copy (Ctrl+C or right click and chose Copy)
5. Close Library and Quit Browser

Actual Results:
Crash reporter pops up

Expected Results:
No crash

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c8be68703225291c63a409516f7a36cdeb6aa314&tochange=122337f76ded825b04a05aa7cc46b2bcd77ea695

Regressed by:
122337f76ded	Makoto Kato — Bug 122337f76ded	Makoto Kato — Bug 1240282 - Don't use NS_LossyConvertUTF16toASCII for URI. r=jimm - Don't use NS_LossyConvertUTF16toASCII for URI. r=jimm

Comment 2

[Marco Bonardo [:mak]]

See also bug 1310430 where this crashes when copying bookmark folders.

Comment 3

[Marco Bonardo [:mak]]

makoto-san, the change seems to introduce a dereference on aUri->GetAsciiSpec(asciiUrl); without checking aURI validity.
LossyCopyUTF16toASCII didn't have this problem cause it was not validating the uri.

Comment 4

[Makoto Kato [:m_kato]]

Attachment: Bug 1310453 - Check whether aUri isn't created.

Since aUri is nullptr, Firefox will crash on shutdown.

Review commit: https://reviewboard.mozilla.org/r/86284/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/86284/

Comment 5

[Marcia Knous [:marcia]]

Tracking 52+ for this shutdown crash which also crashes with a copy operation according to comment 2.

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 8801614 [details]
Bug 1310453 - Check whether aUri isn't created.

https://reviewboard.mozilla.org/r/86284/#review85258

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f0590c71a0a
Check whether aUri isn't created. r=jimm

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3f0590c71a0a

Comment 9

[Makoto Kato [:m_kato]]

Comment on attachment 8801614 [details]
Bug 1310453 - Check whether aUri isn't created.

Approval Request Comment
[Feature/regressing bug #]:
Bug 1240282

[User impact if declined]:
When quitting Firefox, Firefox crash.

[Describe test coverage new/current, TreeHerder]:
Landed in m-c

[Risks and why]:
Low.  Add error check only.

[String/UUID change made/needed]:
No

Comment 10

[Makoto Kato [:m_kato]]

Comment on attachment 8801614 [details]
Bug 1310453 - Check whether aUri isn't created.

Approval Request Comment
[Feature/regressing bug #]:
Bug 1240282

[User impact if declined]:
When quitting Firefox, Firefox crash.

[Describe test coverage new/current, TreeHerder]:
Landed in m-c

[Risks and why]:
Low.  Add error check only.

[String/UUID change made/needed]:
No

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8801614 [details]
Bug 1310453 - Check whether aUri isn't created.

Crash fix, Aurora51+, Beta50+

Comment 12

[Bastien Abadie [:bastien]]

TEST: it should fail.

Comment 13

[Bastien Abadie [:bastien]]

Revert tracking for firefox49.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bcca478fadc0

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f6ee39199d93

Comment 16

[Ciprian Georgiu, Desktop QA]

I've managed to reproduce the crash using one of the the affected builds (Nightly 52.0a1 from 2016-10-15).

This issue is verified fixed on 50.0b9 (2016-10-20), latest Aurora 51.0a2 (2016-10-21) and latest Nightly (2016-10-21) under the following OSes:
- Windows 10 x64
- Ubuntu 16.04 x64 LTS
- Mac OS X 10.11.6