Opened on 11/27/2018 at 09:37:41 PM

Closed on 01/08/2019 at 07:59:02 PM

Last modified on 03/13/2019 at 10:54:00 AM

#7141 closed change (fixed)

Implement abort-current-inline-script snippet

Reported by: hfiguiere Assignee: hfiguiere
Priority: P2 Milestone:
Module: Core Keywords: circumvention
Cc: mjethani, arthur, sdixit Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29953560/

Description (last modified by amrmak)

Background

Implement abort-current-inline-script snippet that will abort an inline script based on an API it accesses and the text content of the script.

While this looks like issue #6983, this snippet runs on Chrome as well.

What to change

Implement a snippet with the following signature:

function abortCurrentInlineScript(api, search = null)

The value of the api parameter may be a string like "window.location.href" or "console.log". If the specified API exists, it should be wrapped such that accessing the API causes the parent script to abort if it is an inline script.

The search parameter may be a string, optionally surrounded by / (slashes) so it is treated as a regular expression. If it is specified, the inline script should be aborted only if it contains the given pattern.

Note for testers

Test 1:

Serve the attached HTML document Test 1 over HTTP(S)

And add the following filter

example.com#$#abort-current-inline-script console.log Hello

(change the domain appropriately)

The console should only have "There can only be one!" printed.

Note: the test might fail if the inline scripts are run before the snippet has a chance to run. The sample document works here, but this is not a guarantee that it is enough to run elsewhere.

Attachments (1)

index.html (3.1 KB) - added by hfiguiere on 01/08/2019 at 07:53:43 PM.
Test 1

Download all attachments as: .zip

Change History (25)

comment:1 Changed on 11/29/2018 at 03:05:07 AM by hfiguiere

  • Owner set to hfiguiere
  • Review URL(s) modified (diff)

comment:2 Changed on 11/29/2018 at 03:05:15 AM by hfiguiere

  • Status changed from new to reviewing

comment:3 Changed on 12/01/2018 at 12:32:03 AM by hfiguiere

  • Description modified (diff)

comment:4 Changed on 12/03/2018 at 11:11:07 AM by mjethani

We could add one more example here:

<!DOCTYPE html>
<body>
 <script>
 console.log("Hello from inline script!");
 </script>
 <script>
 console.log("Hello from inline JavaScript!");
 </script>
 <script>
 console.log("There can only be one!");
 </script>
</body>

And the filter:

example.com#$#abort-current-inline-script console.log '/inline [A-Za-z]*[Ss]cript/'
Last edited on 12/03/2018 at 11:11:18 AM by mjethani

comment:5 Changed on 12/03/2018 at 06:14:44 PM by hfiguiere

  • Description modified (diff)

comment:6 Changed on 12/04/2018 at 10:53:22 AM by mjethani

  • Description modified (diff)

comment:7 Changed on 12/04/2018 at 10:53:55 AM by mjethani

  • Description modified (diff)

Changed on 01/08/2019 at 07:53:43 PM by hfiguiere

Test 1

comment:8 Changed on 01/08/2019 at 07:54:17 PM by hfiguiere

  • Description modified (diff)

comment:9 Changed on 01/08/2019 at 07:56:38 PM by abpbot

A commit referencing this issue has landed:
Issue 7141 - Implement abort-current-inline-script

comment:10 Changed on 01/08/2019 at 07:59:02 PM by hfiguiere

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:11 Changed on 01/09/2019 at 01:35:04 PM by mjethani

  • Priority changed from Unknown to P2
  • Ready set

comment:12 Changed on 01/17/2019 at 08:55:36 AM by abpbot

A commit referencing this issue has landed:
Issue 7141 - Implement abort-current-inline-script

comment:13 Changed on 01/17/2019 at 09:01:36 AM by mjethani

Note: the test might fail if the inline scripts are run before the snippet has a chance to run

Maybe we should add an explicit timeout instead to make the test more reliable? I have done this for other snippets (#7207 also has a timeout).

comment:14 Changed on 01/17/2019 at 09:09:19 AM by mjethani

  • Description modified (diff)

comment:15 Changed on 01/22/2019 at 05:58:23 AM by Ross

Working in the browser versions below. Will verify once able to test on minimum versions.

ABP 3.4.2.2231
Chrome 71.0.3578.98 / Windows 10
Firefox 64.0.2 / Windows 10

comment:16 Changed on 01/22/2019 at 01:54:33 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Implemented and working. Regex works fine.

ABP 3.4.2.2231
Chrome 71.0.3578.98 / Windows 10 (Latest)
Chrome 49.0.2623.75 / Windows 10 (Min supported)
Firefox 64.0.2 / Windows 10 (Latest)
Firefox 51.0 / Windows 10 (Min supported)
Opera 57.0.3098.76 / Windows 10 (Latest)
Opera 36.0.2130.65 / Windows 10 (Min supported)

comment:17 Changed on 01/23/2019 at 02:54:47 AM by mjethani

  • Sensitive unset

comment:18 Changed on 01/24/2019 at 08:55:22 PM by Ross

I have a question about this snippet after playing with it while creating test pages: What counts as an "api" here? Should I be able to define a function in a script tag on the page and target that as well as a function/api provided by the browser?

comment:19 Changed on 01/25/2019 at 07:12:05 AM by mjethani

Yes, it's basically any global property, not necessarily a browser API.

comment:20 Changed on 01/25/2019 at 02:21:07 PM by Ross

Thanks Manish.

comment:21 Changed on 01/25/2019 at 02:21:15 PM by Ross

Does not work in Edge. Should it? I will open a new ticket if so.

ABP 0.9.12.2237
Edge 44.17763.1.0 / EdgeHTML 18.17763 / Windows 10

comment:22 Changed on 01/29/2019 at 09:05:09 AM by mjethani

Let's open a new ticket for Edge, in my opinion.

comment:23 Changed on 02/06/2019 at 04:09:14 PM by hfiguiere

It's possible that it is just a timing issue, as usual. This one is quite sensitive.

Yes please file a bug if you can't make it work on Edge.

comment:24 Changed on 03/13/2019 at 10:54:00 AM by amrmak

  • Description modified (diff)

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.