Closed Bug 1038465 Opened 5 years ago Closed 5 years ago

pass suggested filename to application reputation check

Categories

(Core :: DOM: Security, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox32 --- fixed
firefox33 --- fixed

People

(Reporter: mmc, Assigned: mmc)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

We rely on it for sending remote lookups but aren't actually filling it in.
Assignee: nobody → mmc
Status: NEW → ASSIGNED
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
Attachment #8455836 - Flags: review?(paolo.mozmail)
Attachment #8455836 - Attachment is obsolete: true
Attachment #8455836 - Flags: review?(paolo.mozmail)
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
Attachment #8455895 - Flags: review?(paolo.mozmail)
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.
Attachment #8455895 - Flags: review?(paolo.mozmail) → review+
Thanks, Paolo! Rewrote to omit aSuggestedFilename, and an approximation is good enough for now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7441f4dde492
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
Attachment #8456231 - Flags: review+
Attachment #8456231 - Flags: approval-mozilla-aurora?
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.
Attachment #8456231 - Flags: approval-mozilla-aurora?
Try, now with mochitest and (hopefully) the right pref set: https://tbpl.mozilla.org/?tree=Try&rev=fcb5cadba1d4
https://hg.mozilla.org/mozilla-central/rev/69b8e7c58dc6
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Attachment #8456231 - Attachment is obsolete: true
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
Attachment #8457015 - Attachment description: Pass suggested filename to application reputation query ( → Aurora-specific patch: Pass suggested filename to application reputation query (
Attachment #8457015 - Flags: review+
Attachment #8457015 - Flags: approval-mozilla-aurora?
Whiteboard: in-testsuite+
Attachment #8457015 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: in-testsuite+
Whiteboard: in-testsuite+
You need to log in before you can comment on or make changes to this bug.