Third party filters blocking first party requests

[scuturic]

Environment

Win 8.1
Chrome 54.0.2840.87
Adblock Plus 1.12.4 with only the filter ||apptap.com^$third-party and no subscriptions.

How to reproduce

1. Create a new Chrome tab
2. Type the URL http://www.apptap.com/ into the URL bar.
3. Hit the Enter key.

OR

1. Browse to ​https://static.kzar.co.uk/4599-link-test/
2. Click on the link.

Observed behaviour

The page layout is often broken since the page's stylesheet was blocked. It doesn't happen every time but it does quite often. If you then refresh the page the stylesheet will not be blocked.

Expected behaviour

The page should be displayed correctly, the stylesheet shouldn't be blocked.

Notes

• ​See related discussion in the forums.
• This a regression caused by the commit ​Issue 4386 - Fixed determining document domain, particularly after being redirected.
• The problem is caused by the fact that pages sometimes make web requests before the onCommitted event is being fired.

[mapx]

it seems a lot of sites have the same wrong behaviour:
​http://www.appnext.com/
​http://www.apptap.com/

How to reproduce it:

open a new tab, type www.apptap.com:

• the css file will be blocked
• F5 refresh ==> the page works fine

If you whitelist the site, repeat the steps above ==> the css file blocked again !

Could be a regression while fixing:
1649: 2016-08-31 17:11 +0200 Wladimir Palant
Issue #4386 - Fixed determining document domain, particularly after being redirected
?

see also in forum:
​https://adblockplus.org/forum/viewtopic.php?f=10&t=49528

If you add
@@||apptap.com^$stylesheet

the issue is gone.

There is only a third-party filter in easylist:
||apptap.com^$third-party

[trev]

I'd rather expect #4483 to be the cause.

[kzar]

I can reproduce this too with Chrome 54 on Linux. I agree it's likely to be a regression, going to try and track down what caused this now.

[kzar]

Like Wladimir I expected the recent changes for #4483 to be to blame but it seems Mapx was right and that the changes for #4386 caused this. I can reproduce the problem with the ​Issue 4386 - Fixed determining document domain, particularly after being redirected revision but not before.

[mapx]

yep, I was guessing is 4386 because the symptoms are the same as in the original 4386
"If you whitelist the site, repeat the steps above ==> the css file blocked again"

[kzar]

Edit: I was mistaken, since as ​Sebastian pointed out in the code review Chrome gives pre-rendered pages a new tab id. As for the $sitekey stuff, I've opened #4632 for that.

Unless I'm mistaken it's basically the same problem as #4483.

If a page makes a web request before its onCommitted event has fired the request will be blocked / whitelisted based upon the old page structure. So if the URL for the previous page was of a different domain the request will be considered third party, or if the new page has a sitekey we won't know to take it into account.

Thing is the old behaviour was no better, arguably worse. The onBeforeNavigate event we used wasn't fired after redirections, so the page structure was not updated at all for those. Also it was fired for pre-rendered pages which meant that we effectively assumed that pre-rendered pages would always be navigated to. So we can't simply revert #4386 and I think it's a little unfair to consider this issue a regression caused by that change.

Ultimately I think we need to rethink the page structure logic so that pre-rendered pages can be accounted for properly. While we're at it I think adding unit tests for how the page structure is maintained based upon navigation events would be a pretty good idea. I don't think this is the right time to do that however since in the near future we plan to remove some of the Page abstractions (e.g. #4580) and other ext code now that Safari support has been dropped. I think it would make more sense to wait until after that work has been done.

So what can we do for now? Well how about we update the page structure both on onBeforeNavigate and onCommitted. We could remove the workaround for #4483 while at it and look at maintaining the page structure properly after the ext code has been removed. This would mostly be correct and not require major changes.

(Updating the page structure for pre-rendered pages is incorrect but perhaps that doesn't matter too much. One pre-rendered page will replace another until the user eventually has finished typing the URL. The latest guess should always be handled correctly. The only problem would be if the user stopped typing the URL and then continued to use the existing page without navigating at all. Note this was the case before #4483 landed so apparently not too much of a problem.)

[kzar]

(Dang, looks like I was at least partially mistaken since the $sitekey whitelisting for requests directly after a redirect broke when I took the #4483 workaround out. I'll see if the approach otherwise works now.)

[kzar]

(Turns out this is not only for pre-rendered pages.)

[kzar]

(I filed ​Chromium issue 665843 for the underlying problem that pages and frames can make requests before their onCommitted event has fired.)

[mapx]

​https://adblockplus.org/forum/viewtopic.php?f=10&t=49971#p162612
​https://adblockplus.org/forum/viewtopic.php?f=10&t=49937

[mapx]

​https://adblockplus.org/forum/viewtopic.php?f=10&t=50027

[arthur]

Is this the same issue?

Copy this URL ​https://advertise.isleofskye.com/, paste it into the address bar and press enter. Some resources are being blocked while they shouldn't. It's fine if you reload the page.

[kzar]

Yes, that sounds like the same issue.

[8ball]

I have this issue on ABP for CHROME and OPERA - upon first load of my websites homepage my adsense advert is being blocked even when disabled on the site, if i refresh the page it displays...all other ads on website display fine.. just the homepage one doesnt

[kzar]

A commit referencing this issue has landed:
​Issue 4598,4599,4647,4804 - Work around onCommitted flaw

[Ross]

Fixed.

ABP 1.12.4.1739
Chrome 49 / 56 / Windows 10
Chrome 56 / OS X 10.12
Chrome 56 / Ubuntu 16.04
Opera 37 / 41 / Windows 7
Safari 10 / OS X 10.12