Closed
Bug 473345
Opened 16 years ago
Closed 16 years ago
check dom workers against the url-classifier
Categories
(Toolkit :: Safe Browsing, defect, P1)
Toolkit
Safe Browsing
Tracking
()
VERIFIED
FIXED
People
(Reporter: dcamp, Assigned: dcamp)
Details
(Keywords: verified1.9.1)
Attachments
(1 file, 3 obsolete files)
|
(deleted),
patch
|
Details | Diff | Splinter Review |
As mentioned in bug 441359, a new script loader was added that we need to pass through the classifier.
Flags: blocking-firefox3.1?
Attachment #356685 -
Flags: superreview?(jonas)
Attachment #356685 -
Flags: review?(jonas)
|
||
Comment 1•16 years ago
|
||
Jonas: a P1 blocker is waiting on this review.
Flags: blocking-firefox3.1? → blocking-firefox3.1+
Priority: -- → P1
Comment on attachment 356685 [details] [diff] [review]
fix
Looks good, but bent should have a look.
The one thing that would be good to do is to make sure that importScript (only available inside worker) also gets blocked appropriately.
Attachment #356685 -
Flags: superreview?(jonas)
Attachment #356685 -
Flags: superreview+
Attachment #356685 -
Flags: review?(jonas)
Attachment #356685 -
Flags: review?(bent.mozilla)
Comment on attachment 356685 [details] [diff] [review]
fix
This looks good! I agree with Jonas, it would be nice to have a test for an 'importScripts("blockedScript.js")' within the worker.
Attachment #356685 -
Flags: review?(bent.mozilla) → review+
|
||
Comment 4•16 years ago
|
||
I'll certainly take on writing a Mozmill test for this case, could you be a little more specific about the kind of blocking you are talking about?
This is what I'm thinking, blockedScript.js has a self executing function defined with a giant while loop or sync xhr call and after its done a flag is set. If the line after importScripts is executed before that flag is set then we have a failure. Does this sound like reasonable validation? Any suggest improvements?
importScripts should throw if the load is blocked, so maybe ensuring that we see an exception is sufficient.
|
Assignee | |
Comment 6•16 years ago
|
||
This adds a mochitest for importScripts to the existing mochitest
Attachment #357054 -
Flags: review?(bent.mozilla)
|
|
||
Comment 7•16 years ago
|
||
Ah, that's simple enough. Will update when I have my test finished.
|
|
Assignee | |
Comment 8•16 years ago
|
||
Attachment #357054 -
Attachment is obsolete: true
Attachment #357062 -
Flags: review?(bent.mozilla)
Attachment #357054 -
Flags: review?(bent.mozilla)
Comment on attachment 357062 [details] [diff] [review]
now with cleanWorker.js
Awesome, thanks!
Attachment #357062 -
Flags: review?(bent.mozilla) → review+
|
|
Assignee | |
Comment 10•16 years ago
|
||
Attachment #356685 -
Attachment is obsolete: true
Attachment #357062 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 11•16 years ago
|
||
|
||
Updated•15 years ago
|
Flags: in-testsuite+
|
|
||
Comment 12•15 years ago
|
||
verified via code inspection.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
|
||
Updated•10 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•