1038465 - pass suggested filename to application reputation check

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[[:mmc] Monica Chew (no longer reading bugmail)]

We rely on it for sending remote lookups but aren't actually filling it in.

Comment 1

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Pass suggested filename to application reputation query (

Comment 2

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8455836 [details] [diff] [review]
Pass suggested filename to application reputation query (

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

This patch is not perfect but works in many cases. A more completion solution would be to sniff the content-type, etc. but since Chrome is doing something very similar, I think this is good enough for now:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/safe_browsing/download_protection_service.cc&rcl=1405347508&l=463

Comment 3

[[:mmc] Monica Chew (no longer reading bugmail)]

https://tbpl.mozilla.org/?tree=Try&rev=f07d058006a3

Comment 4

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Pass suggested filename to application reputation query (

Comment 5

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8455895 [details] [diff] [review]
Pass suggested filename to application reputation query (

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

https://tbpl.mozilla.org/?tree=Try&rev=7b68b05dc69f

Comment 6

[:Paolo Amadini]

Comment on attachment 8455895 [details] [diff] [review]
Pass suggested filename to application reputation query (

aSuggestedFileName should be just suggestedFileName.

This makes the current code work, hence the r+, but note that the information we're sending is not really the "suggested file name" that is expected by the interface (Content-Disposition filename or URL-inferred filename), but the string the user typed during file name selection, or the default file name including any duplicate counter (for example "Setup (1).exe") in case the user did not change the default.

In rare cases we might do a wrong executability check in case the file is saved without an extension on Windows.

If this works for you, it works for me too :-) But keep in mind that it is an approximation of the ideal behavior. Getting the originally suggested file name would require various changes to the download code.

Comment 7

[[:mmc] Monica Chew (no longer reading bugmail)]

Thanks, Paolo! Rewrote to omit aSuggestedFilename, and an approximation is good enough for now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7441f4dde492

Comment 8

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Pass suggested filename to application reputation query (

Comment 9

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8456231 [details] [diff] [review]
Pass suggested filename to application reputation query (

Approval Request Comment
[Feature/regressing bug #]: Incomplete initial implementation

[User impact if declined]: Remote lookups not sent to application reputation service until 33

[Describe test coverage new/current, TBPL]: toolkit/components/downloads/test/unit/test_app_rep*.js, https://tbpl.mozilla.org/?tree=Try&rev=7b68b05dc69f

[Risks and why]: Low risk

[String/UUID change made/needed]: none

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Backed out for hitting the network during tests (fatal since bug 995417 landed about a month ago).
https://hg.mozilla.org/integration/mozilla-inbound/rev/83a246bb7860

https://tbpl.mozilla.org/php/getParsedLog.php?id=43837300&tree=Mozilla-Inbound

Comment 11

[Ryan VanderMeulen [:RyanVM]]

The relevant part of that log:
09:06:23     INFO -  Non-local network connections are disabled and a connection attempt to sb-ssl.google.com (173.194.33.169) was made.  You should only access hostnames available via the test networking proxy (if running mochitests) or from a test-specific httpd.js server (if running xpcshell tests). Browser services should be disabled or redirected to a local server.

Comment 12

[[:mmc] Monica Chew (no longer reading bugmail)]

Try, now with mochitest and (hopefully) the right pref set: https://tbpl.mozilla.org/?tree=Try&rev=fcb5cadba1d4

Comment 13

[[:mmc] Monica Chew (no longer reading bugmail)]

https://tbpl.mozilla.org/?tree=Try&rev=1cd8117e4167

Comment 14

[[:mmc] Monica Chew (no longer reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/69b8e7c58dc6

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/69b8e7c58dc6

Comment 16

[[:mmc] Monica Chew (no longer reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/80af4d90c3d5

Comment 17

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Aurora-specific patch: Pass suggested filename to application reputation query (

Comment 18

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8457015 [details] [diff] [review]
Aurora-specific patch: Pass suggested filename to application reputation query (


Approval Request Comment
[Feature/regressing bug #]: Incomplete initial implementation

[User impact if declined]: Remote lookups not sent to application reputation service until 33

[Describe test coverage new/current, TBPL]: toolkit/components/downloads/test/unit/test_app_rep*.js, https://hg.mozilla.org/mozilla-central/rev/69b8e7c58dc6

[Risks and why]: Low risk

[String/UUID change made/needed]: none

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/80af4d90c3d5

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5f975bb5e02c