1165816 - Cancel remote application reputation request after a certain timeout

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect)

Description

[Hector Zhao [:hectorz]]

As a result of the unreliable connections to any Google service, many users in China are experiencing a long delay (in minutes) before a downloaded executable file can be marked as safe/blocked.

Beijing office would like to request introducing a timeout for application reputation check, similar to what happened in bug 1024555 for safe browsing gethash request, thanks!

Comment 1

[François Marier [:francois]]

I'll add a similarly-configurable timeout of 5 seconds.

Comment 3

[François Marier [:francois]]

Attachment: Stalling server in node.js

Here's a test case to easily reproduce this bug.

Comment 4

[François Marier [:francois]]

Attachment: MozReview Request: bz://1165816/francois

/r/9239 - Bug 1165816 - Cancel remote application reputation requests after a certain timeout

Pull down this commit:

hg pull -r fcd4d3c590da5749760c48921afc47a71531e78f https://reviewboard-hg.mozilla.org/gecko/

Comment 5

[François Marier [:francois]]

Comment on attachment 8609152 [details]
MozReview Request: bz://1165816/francois

/r/9239 - Bug 1165816 - Cancel remote application reputation requests after a certain timeout

Pull down this commit:

hg pull -r fcd4d3c590da5749760c48921afc47a71531e78f https://reviewboard-hg.mozilla.org/gecko/

Comment 6

[François Marier [:francois]]

Comment on attachment 8609152 [details]
MozReview Request: bz://1165816/francois

/r/9239 - Bug 1165816 - Cancel remote application reputation requests after a certain timeout

Pull down this commit:

hg pull -r d0a637cdb6698deebc0fe02b64139d9d8f36371c https://reviewboard-hg.mozilla.org/gecko/

Comment 7

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8609152 [details]
MozReview Request: bz://1165816/francois

https://reviewboard.mozilla.org/r/9237/#review7925

Comment 8

[Gian-Carlo Pascutto [:gcp]]

https://reviewboard.mozilla.org/r/9239/#review7931

Ship It!

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b068592c488a

Comment 10

[François Marier [:francois]]

As per Google's suggestion, I have set the default timeout to 10 seconds.

Comment 11

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/ad3297e3ffd8 for Windows debug unexpected assertions in webide tests like https://treeherder.mozilla.org/logviewer.html#?job_id=10107110&repo=mozilla-inbound

Comment 12

[François Marier [:francois]]

Attachment: MozReview Request: Bug 1165816 - Cancel remote application reputation requests after a certain timeout. r=gcp

Bug 1165816 - Cancel remote application reputation requests after a certain timeout. r=gcp

Comment 13

[François Marier [:francois]]

This new revision of the patch fixes the assertion error in Windows debug builds:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ab17fcff34b

Comment 14

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8616639 [details]
MozReview Request: Bug 1165816 - Cancel remote application reputation requests after a certain timeout. r=gcp

https://reviewboard.mozilla.org/r/9239/#review9181

::: toolkit/components/downloads/ApplicationReputation.cpp:755
(Diff revisions 2 - 3)
> +    mTimeoutTimer->Cancel();

Maybe null that pointer here?

Comment 15

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8616639 [details]
MozReview Request: Bug 1165816 - Cancel remote application reputation requests after a certain timeout. r=gcp

https://reviewboard.mozilla.org/r/9239/#review9183

Ship It!

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5389f1e4087c

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5389f1e4087c

Comment 19

[Matt Wobensmith [:mwobensmith][:matt:]]

The links to malware on http://testsafebrowsing.appspot.com are no longer blocked, sadly, and I have no idea why... nor do I know how else to verify that this has been fixed.

Comment 20

[Gian-Carlo Pascutto [:gcp]]

(In reply to Matt Wobensmith from comment #19)
> The links to malware on http://testsafebrowsing.appspot.com are no longer
> blocked, sadly, and I have no idea why... nor do I know how else to verify
> that this has been fixed.

That's a problem. I can't reproduce that though, if I let my Nightly running overnight so it can update the DB, I get the blocks.

Are you on a new profile?

Comment 21

[Matt Wobensmith [:mwobensmith][:matt:]]

Yes, this is with a new profile that has been given plenty of time to download the list.

I believe all of us working on this feature have experienced intermittent issues w/r/t blocking these links, but no one has an explanation. 

I will dig into this deeper in the next week.

Comment 22

[Matt Wobensmith [:mwobensmith][:matt:]]

Today I was able to verify that this was fixed, using the links on that page. I verified using yesterday's Nightly Fx41, against an affected Nightly from 2015-05-17. 

Again, I don't know why this intermittently fails, but for the purposes of this bug, I would consider it fixed.

I still plan on investigating the intermittent failure.