Closed Bug 1029781 Opened 6 years ago Closed 6 years ago

image loaders should set loadFlags | NS_LOAD_CLASSIFY_URI

Categories

(Core :: DOM: Security, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: mmc, Assigned: mmc)

References

Details

Attachments

(1 file)

Since images can hit the malware/phishing/tracking lists, they should go through the channel classifier.
Assignee: nobody → mmc
Status: NEW → ASSIGNED
Comment on attachment 8445535 [details] [diff] [review]
Set NS_LOAD_CLASSIFY_URI in image loader

Review of attachment 8445535 [details] [diff] [review]:
-----------------------------------------------------------------

Hey Seth,

This change forces images to go through nsChannelClassifier. This is the correct thing to do for phishing and malware checks, but also we especially need it for tracking protection (which will be prefed off by default), since webbugs are often used for tracking. Generally safebrowsing lookups are very fast (<< .1ms) but it would be great to cross check for performance regressions if such metrics exist.

http://telemetry.mozilla.org/#filter=nightly%2F33%2FURLCLASSIFIER_LOOKUP_TIME&aggregates=multiselect-all!Submissions!Mean!5th%20percentile!25th%20percentile!median!75th%20percentile!95th%20percentile&evoOver=Builds&locked=true&sanitize=true&renderhistogram=Graph

Monica
Attachment #8445535 - Flags: review?(seth)
Attachment #8445535 - Flags: review?(seth) → review+
Thanks Wes. Working on it.
Flags: needinfo?(mmc)
Hey gcp, this seems to be breaking on Classifier::Open:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#745

which is using the profile directory. I'm a little confused, because this breakage is on Android only and I thought that safebrowsing worked on Android. Do I need to add do_get_profile() to these tests, or fix #ifdef MOZ_CLASSIFIER somewhere?
Flags: needinfo?(gpascutto)
My best bet is that you're causing Classifier to be initialized earlier or be activated in some tests where it wasn't before. So yeah, problems with the profile seem likely.

I see no reason why this would trigger on Android only.
Flags: needinfo?(gpascutto)
https://hg.mozilla.org/mozilla-central/rev/f3192b2f9195
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Depends on: 1164518
You need to log in before you can comment on or make changes to this bug.