Closed Bug 394525 Opened 17 years ago Closed 17 years ago

malware check non-http URIs

Categories

(Core :: DOM: Navigation, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta2

People

(Reporter: dcamp, Assigned: dcamp)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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().
Flags: blocking1.9?
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 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+
Flags: blocking1.9? → blocking1.9+
Priority: -- → P1
Attachment #280061 - Flags: superreview?(cbiesinger) → superreview+
Comment on attachment 280061 [details] [diff] [review]
treat Suspend() failures as nonfatal

Please queue this for landing ASAP. Thanks.
Attachment #280061 - Flags: approval1.9+
Keywords: checkin-needed
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
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 → ---
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 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+
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
(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.
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
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Blocks: 812649
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: