Closed
Bug 394525
Opened 17 years ago
Closed 17 years ago
malware check non-http URIs
Categories
(Core :: DOM: Navigation, defect, P1)
Core
DOM: Navigation
Tracking
()
RESOLVED
FIXED
mozilla1.9beta2
People
(Reporter: dcamp, Assigned: dcamp)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
2.25 KB,
patch
|
bzbarsky
:
review+
Biesinger
:
superreview+
sayrer
:
approval1.9+
|
Details | Diff | Splinter Review |
3.10 KB,
patch
|
tony
:
review+
|
Details | Diff | Splinter Review |
Right now the docshell only malware checks http URIs, because we're not sure other channels properly implement Suspend() right after AsyncOpen(). We should probably: a) Treat failures from Suspend() as non-fatal b) Check to make sure the existing channels can be suspended before OnStartRequest().
Updated•17 years ago
|
Flags: blocking1.9?
Assignee | ||
Comment 1•17 years ago
|
||
This patch restores the old NS_URIChainHasFlags() checks and treats Suspend() failures as nonfatal.
Assignee: nobody → dcamp
Status: NEW → ASSIGNED
Attachment #280061 -
Flags: superreview?(cbiesinger)
Attachment #280061 -
Flags: review?(bzbarsky)
Comment 2•17 years ago
|
||
Comment on attachment 280061 [details] [diff] [review] treat Suspend() failures as nonfatal Is there a bug on the nsJSChannel issue?
Attachment #280061 -
Flags: review?(bzbarsky) → review+
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Updated•17 years ago
|
Priority: -- → P1
Updated•17 years ago
|
Attachment #280061 -
Flags: superreview?(cbiesinger) → superreview+
Comment 3•17 years ago
|
||
Comment on attachment 280061 [details] [diff] [review] treat Suspend() failures as nonfatal Please queue this for landing ASAP. Thanks.
Attachment #280061 -
Flags: approval1.9+
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Comment 4•17 years ago
|
||
Checking in docshell/base/nsDocShell.cpp; /cvsroot/mozilla/docshell/base/nsDocShell.cpp,v <-- nsDocShell.cpp new revision: 1.867; previous revision: 1.866 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Summary: malware check non-http URIs. → malware check non-http URIs
Target Milestone: --- → mozilla1.9 M10
Comment 5•17 years ago
|
||
This patch caused the unit test for bug 377539 to fail, so this patch was backed out. Reopening. *** 9756 ERROR FAIL | iframe got wrong screen.width | got -1, expected 1280 | /tests/dom/tests/mochitest/bugs/test_bug377539.html
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 6•17 years ago
|
||
The patch was triggering a race condition in that unit test - I filed bug 405479 about that race. That said, we shouldn't be trying to check data: uris at all, or any uri that doesn't have a hostname. This patch disables malware checking for such uris.
Attachment #290259 -
Flags: review?(tony)
Comment 7•17 years ago
|
||
Comment on attachment 290259 [details] [diff] [review] don't classify URIs with no hostname Looks good, although it's unfortunate that wysiwyg: and data: get a free pass.
Attachment #290259 -
Flags: review?(tony) → review+
Assignee | ||
Comment 8•17 years ago
|
||
Comment on attachment 290259 [details] [diff] [review] don't classify URIs with no hostname Checking in src/nsUrlClassifierDBService.cpp; /cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp,v <-- nsUrlClassifierDBService.cpp new revision: 1.38; previous revision: 1.37 done Checking in src/nsUrlClassifierUtils.cpp; /cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierUtils.cpp,v <-- nsUrlClassifierUtils.cpp new revision: 1.9; previous revision: 1.8 done Checking in tests/unit/test_dbservice.js; /cvsroot/mozilla/toolkit/components/url-classifier/tests/unit/test_dbservice.js,v <-- test_dbservice.js new revision: 1.7; previous revision: 1.6 done
Assignee | ||
Comment 9•17 years ago
|
||
(In reply to comment #7) > (From update of attachment 290259 [details] [diff] [review]) > Looks good, although it's unfortunate that wysiwyg: and data: get a free pass. To clarify, a URI with no hostname will never match the blacklist anyway, because it depends on the value of the hostname.
Assignee | ||
Comment 10•17 years ago
|
||
Comment on attachment 280061 [details] [diff] [review] treat Suspend() failures as nonfatal Checking in nsDocShell.cpp; /cvsroot/mozilla/docshell/base/nsDocShell.cpp,v <-- nsDocShell.cpp new revision: 1.872; previous revision: 1.871 done
Assignee | ||
Updated•17 years ago
|
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•