Closed Bug 713485 Opened 13 years ago Closed 13 years ago

Crash when invalid drive is used in local file URL

Categories

(Core :: Widget: Win32, defect)

Product:
Core
Component:
Widget: Win32
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla13

People

(Reporter: attekett, Assigned: bbondy)

Details

(Keywords: crash)

Crash Data

Attachments

(3 files, 1 obsolete file)

634604657471421591.html
(deleted), text/html
Details
sample html
(deleted), text/html
Details
Patch v2.
(deleted), patch
neil
: review+
Details | Diff | Splinter Review
Attached file 634604657471421591.html (deleted) —
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:12.0a1) Gecko/20111224 Firefox/12.0a1 Build ID: 20111224031026 Steps to reproduce: Open attachment file in Nightly build firefox. I'll reduce the repro-case after the Holidays. Actual results: Crash
Attachment #584297 - Attachment mime type: text/plain → text/html
I can confirm a crash if I load the testcase via local filesystem. This seems to be the regression range: Last good nightly: 2011-09-21 First bad nightly: 2011-09-22 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e8bd19f6abbb&tochan ge=4495e1f795c2
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash, regression
Product: Firefox → Core
QA Contact: general → general
Version: 12 Branch → Trunk
The crash report is useless: bp-f0aa99af-c05c-4735-9e50-52ced2111225 except that it confirms EXCEPTION_NONCONTINUABLE_EXCEPTION
Regression window(m-c): Works: http://hg.mozilla.org/mozilla-central/rev/e8bd19f6abbb Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110921 Firefox/9.0a1 ID:20110921030906 Fails: http://hg.mozilla.org/mozilla-central/rev/3178f1c42505 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110921 Firefox/9.0a1 ID:20110921013649 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e8bd19f6abbb&tochange=3178f1c42505 Regression window(m-c): Works: http://hg.mozilla.org/integration/mozilla-inbound/rev/1e41259daf67 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110920 Firefox/9.0a1 ID:20110920121748 Fails: http://hg.mozilla.org/integration/mozilla-inbound/rev/6bcc3a001bb3 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110920 Firefox/9.0a1 ID:20110920140549 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1e41259daf67&tochange=6bcc3a001bb3 Triggered by: c237a8550070 Boris Zbarsky — Bug 444641 part 3. Propagate the loading principal to the image channel as needed. r=joe,dveditz
Blocks: 444641
bp-fd7eaf2f-3039-405c-9cdd-8b2bb2111226
Crash Signature: [@ msvcr80.dll@0x260f7]
Reduced test-case is file with the following content: <BODY background="/$:"/> Results: https://crash-stats.mozilla.com/report/index/bp-141fb79b-cfd9-4c64-a5ba-668502111226
I can't reproduce the crash in a trunk build on Mac. Is this Windows-only? The crash report stack is obviously busted, sadly. :( But I can buy us crashing on a null-deref here. Alice0775, do you happen to have a debug build by any chance? Accurate information on where this is actually crashing would be really useful!
Can't reproduce on Ubuntu 11.04 x64 either. Seems like windows only. Can someone clarify the noncontinuable exception for me? Never faced one before.
I don't think it's worth worrying about the exception type. Given the crash address, this is a null-deref; given the Windows-only thing and the testcase, it's somewhere in nsLocalFileWin. The question is where....
Attached file sample html (deleted) —
bp-461a2f5b-b2d1-45cf-b0f3-007322111226 bp-5159f007-b54a-4846-b596-5b3342111226 $: also crashes on Firefox3.0-12.0
No longer blocks: 444641
Keywords: regression
Those stacks are still showing crashes at points where there is nothing there to crash. :(
Component: General → Widget: Win32
QA Contact: general → win32
> int drive = TOUPPER(path.First()) - 'A' + 1; Obviously |drive| will have a bogus value when the drive letter is invalid.
Assignee: nobody → netzen
Summary: msvcr80.dll EXCEPTION_NONCONTINUABLE_EXCEPTION → Crash when invalid drive is used in local file URL
Attached patch Patch v1. (obsolete) (deleted) — — Splinter Review
Attachment #593302 - Flags: review?(neil)
Indeed, running the test case in a debug build is much more effective: Microsoft Visual C++ Debug Library Debug Assertion Failed! Program: ... File: ...\crt\src\getcwd.c Line: 233 Expression: ("Invalid Drive", 0) Sadly Gecko then hangs because it executes a posted runnable that tries to XHR to perform an addon check that tries to create the bad cert handler for the first time by loading it from the startup cache even though I used -purgecaches which deadlocks when the startup cache thread tries to access the CRT...
Actually what worries me is why we are hitting this code at all because file:///C:/ should be a canonical path...
At this point in code we're no longer working with file URLs but actual file paths. nsPrincipal::CheckMayLoad calls fileURL->GetFile() to get the targetFile, and then calls Normalize on that target file.
Yes, I do understand that Normalize has been called, but C:\ is already canonical, so we shouldn't be trying to canonicalise it in this case. The easiest example I could find of this is that launching composer using seamonkey -edit C:\ actually loads the listing for the current directory!
I understand your concern and I'd like to investigate that in the context of another bug since it isn't related to this crash. For example this crash can be reproduced by anyone using nsIFile and so this fix should be applied independently. Please let me know if you disagree.
Comment on attachment 593302 [details] [diff] [review] Patch v1. In that case I would prefer it if InitWithPath failed immediately.
Attachment #593302 - Flags: review?(neil)
Attached patch Patch v2. (deleted) — — Splinter Review
Thanks for the review. Implemented review comments.
Attachment #593302 - Attachment is obsolete: true
Attachment #594698 - Flags: review?(neil)
In case it gets an r+, this is passing try.
Attachment #594698 - Flags: review?(neil) → review+
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/094e4a9472a1
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: