Opened on 04/15/2017 at 02:31:11 PM

Closed on 08/29/2019 at 05:43:52 PM

#5146 closed change (rejected)

[emscripten] Implement DownloadableSubscription.parseDownload()

Reported by: trev Assignee: hfiguiere
Priority: P3 Milestone:
Module: Core Keywords: closed-in-favor-of-gitlab
Cc: Blocked By: #5137
Blocking: #4122 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29606600/
https://codereview.adblockplus.org/29630576/
https://gitlab.com/eyeo/adblockplus/adblockpluscore/merge_requests/6

Description (last modified by hfiguiere)

Background

In order to leave all text processing in C++ code, we need to process filter list downloads here as well.

What to change

Implement a new method DownloadableSubscription.parseDownload() as an extended replacement of FilterStorage.updateSubscriptionFilters(), to be called from Synchronizer._onDownloadSuccess and implement most of its logic. The method should return a _DownloadableSubscription_Parser instance:

  • buffer property is a pointer to the input buffer and bufferSize property its size (64kB?).
  • The buffer should be filled with lines of data followed by line breaks, until there is not enough space left for the next line or no more lines.
  • process(numChars) should be called to process the current buffer.
  • finalize() should be called in order to replace subscription filters. If delete() is called before finalize() the entire state is discarded and there is no effect on the subscription object.

There is a number of special cases here:

  • Filter list header: if invalid, subscription status should be set to synchronize_invalid_data and further input ignored - no filter replacement. Otherwise the version number from the header should be saved in requiredVersion property.
  • ! Checksum: comment: not saved as filter, checksum validation should be performed (MD5 implementation required). If it fails, status should be set to synchronize_checksum_mismatch and the filters not replaced. obsolete with issue 6849.
  • ! Redirect, ! Homepage, ! Title comments: value should be saved in the corresponding subscription field. This field needs to be added for redirects, its value should not be serialized and should be used as an indicator for Synchronizer that it needs to handle a redirect.
  • ! Expires comment: the value should be returned by finalize() so that Synchronizer can update the expiration data properly. If finalize() returns 0 the default expiration interval is used.
  • If filters are replaced, a notification with topic subscription.beforefiltersreplaced should be sent out before the list of filter changes and another with topic subscription.filtersreplaced after the new filters are in place. These notifications replace current subscription.updated notification.

Attachments (0)

Change History (9)

comment:1 Changed on 04/15/2017 at 02:37:33 PM by trev

  • Description modified (diff)

comment:2 Changed on 05/18/2017 at 02:34:20 PM by trev

  • Blocked By 5137 added

comment:3 Changed on 11/10/2017 at 02:25:04 PM by hfiguiere

  • Owner set to hfiguiere

comment:4 Changed on 11/13/2017 at 10:35:54 PM by hfiguiere

  • Review URL(s) modified (diff)

comment:5 Changed on 11/18/2017 at 05:52:43 AM by oleksandr

  • Status changed from new to reviewing

comment:6 Changed on 12/05/2017 at 09:22:52 AM by hfiguiere

  • Review URL(s) modified (diff)

comment:7 Changed on 08/14/2018 at 03:40:27 AM by hfiguiere

  • Description modified (diff)

With issue 6849 and 6850 we don't need to deal with checksum anymore.

comment:8 Changed on 11/13/2018 at 08:15:30 PM by hfiguiere

  • Review URL(s) modified (diff)

comment:9 Changed on 08/29/2019 at 05:43:52 PM by sebastian

  • Keywords closed-in-favor-of-gitlab added
  • Resolution set to rejected
  • Status changed from reviewing to closed

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from hfiguiere.
 
Note: See TracTickets for help on using tickets.