Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#537 closed change (fixed)

[Downloader] Mark second downloads

Reported by: fhd Assignee: greiner
Priority: P2 Milestone: Adblock-Plus-2.6.7-for-Firefox
Module: Core Keywords: 2014q4
Cc: trev, greiner Blocked By:
Blocking: #536 Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/6013081427640320/
http://codereview.adblockplus.org/4848713646211072/

Description (last modified by trev)

Background

See #536.

What to change

Downloads made for a resource should add the parameter downloadCount which needs to be incremented for each download and max out at 5+.

Hints for testers

Filter subscription updates should work the same as before, without any user visible changes. The download URLs will have an additional parameter.

Change History (17)

comment:1 Changed 5 years ago by fhd

  • Cc trev added

comment:2 Changed 5 years ago by fhd

  • Description modified (diff)

comment:3 Changed 5 years ago by greiner

  • Platform set to Unknown

What about making this more generic and instead of a secondDownload=true add a downloadCount=12 parameter or overloading the lastVersion one (e.g. lastVersion=12/201410060850)? This increases the request's entropy and could allow us to make out usage patterns which at a later point we could use to detect abnormalities.

comment:4 Changed 5 years ago by trev

I would rather not collect more data than absolutely necessary here. However, I am also afraid that anomalies might not stop at the initial download. So something like downloadCount=5+ might make sense (counting to 5 downloads and sending only 5+ after that). Larger download counts might allow recognizing individual users here and we definitely don't want that.

Note that the additional entropy is also a problem for our server-side processing, it makes caching more problematic.

comment:5 Changed 5 years ago by fhd

  • Keywords 2014q4 added

comment:6 Changed 5 years ago by greiner

  • Cc greiner added

comment:7 Changed 5 years ago by greiner

  • Description modified (diff)
  • Owner set to greiner

comment:8 Changed 5 years ago by greiner

Actually, one important detail that's missing for implementation of the downloadCount=5+ approach is where to store the counter. In the initially suggested approach the second download would be determined implicitly by checking for the last version to be zero.

Therefore I see the following options:

  • Implement only for second downloads (problem: won't allow us to distinguish downloads 3-5+)
  • Attach counter to downloadables (problem: we currently don't store downloadables)
  • Attach counter to notification data and subscriptions (problem: problematic to reuse code)

None of these options is optimal so let me know if you can think of a better way to store the counter. Otherwise we might want to go with the original approach but for the sake of forwards-compatibility I'd recommend sticking with downloadCount instead of secondDownload.

comment:9 Changed 5 years ago by trev

In the initially suggested approach the second download would be determined implicitly by checking for the last version to be zero.

No, you cannot distinguish first and second download via lastVersion.

Implement only for second downloads (problem: won't allow us to distinguish downloads 3-5+)

Won't help here, see above.

Attach counter to downloadables (problem: we currently don't store downloadables)

No, downloadables aren't stored directly. They will need the download count as an additional field however.

Attach counter to notification data and subscriptions (problem: problematic to reuse code)

Yes, that's the approach to be chosen here - same as for lastVersion and a bunch of similar fields.

comment:10 Changed 5 years ago by greiner

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:11 Changed 5 years ago by greiner

  • Review URL(s) modified (diff)

comment:12 Changed 5 years ago by greiner

  • Milestone set to Adblock-Plus-for-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:13 Changed 5 years ago by sven

  • Keywords 2014q4 removed

comment:14 Changed 5 years ago by fhd

  • Keywords 2014q4 added

comment:15 Changed 5 years ago by fhd

  • Keywords 2014q4 removed

comment:16 Changed 5 years ago by fhd

  • Keywords 2014q4 added

comment:17 Changed 5 years ago by trev

  • Description modified (diff)
Note: See TracTickets for help on using tickets.