Opened 11 months ago

Last modified 10 months ago

#7305 closed defect

Requests blocked in whitelisted frame — at Version 3

Reported by: greiner Assignee:
Priority: P2 Milestone: Adblock-Plus-3.5.1-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: kzar, sebastian, Ross, arthur Blocked By:
Blocking: Platform: Chrome
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://gitlab.com/eyeo/adblockplus/adblockpluschrome/merge_requests/43
https://gitlab.com/eyeo/adblockplus/adblockpluschrome/merge_requests/49
https://gitlab.com/eyeo/adblockplus/adblockpluschrome/merge_requests/50

Description (last modified by greiner)

Environment

Ubuntu 16.04
Chrome 71
Adblock Plus 3.4.3

How to reproduce

  1. Open http://hbx-stage.media.net/ebaumsworld.php?com_aax_test=161 in an incognito tab
  2. Open Adblock Plus DevTools panel
  3. Reload page and repeat until request to http://s2s.adpushup.com/AdWebService/feedback shows up in Network tab

Observed behavior

Expected behavior

  • Request to http://s2s.adpushup.com/AdWebService/feedback is not blocked (doesn't show up in list due to one of its parent frames being whitelisted by @@http://$document,sitekey=MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAMLlOP3Rke738aeqtDCGp0IgSY5XBv7c+brDMmurbYvOFgakGw6sUG8fwt6VkjnOX9s9Kba1Drg2M9Bye/F3x7MCAwEAAQ filter)
  • Ad is shown on page

What to change

When encountering a request for which ext.getFrame() returns undefined but which specifies both a frameId and parentFrameId property, create a new about:blank frame for the one the request originates from and add it to the frame hierarchy.

For example, this could be achieved by extending ext.getFrame().

Further information

The given example page includes an AAX iFrame whose entire content is supposed to be whitelisted using a $sitekey filter. Inside this frame the ad is served and rendered using third-party ad code.

Ticket is set to confidential because AAX requested to keep example URL private and I'm not aware of any other URLs where this issue occurs.

Logging out the first parameter (frame ID) in updatePageFrameStructure() in ext/background.js leads to the below results.

master (i.e. only frame IDs encountered by frame requests)
0
1460
1461
1462
1463
1465

proposed change
0
-1
1460
1461
1462
1463
1464
1465
1466

There was also an issue with onHeadersReceived not notifying us of all frame requests when types: ["main_frame", "sub_frame"] option is specified but I wasn't able to reproduce it again when coming back to it. If this issue still persists, this may be an additional optimization to avoid adding frames to the hierarchy within onBeforeRequest where performance is critical.

Change History (3)

comment:1 Changed 11 months ago by greiner

One more thing to add is that I'm not quite sure why the browser doesn't notify us of certain anonymous frames. My best bet would be the usage of document.open("text/html", "replace") which results in no history entry being created, possibly in combination with other similar bad practices by any of the code involved in serving/rendering the ad.

comment:2 Changed 11 months ago by kzar

  • Cc sebastian added

Would you mind adding some explanation to the issue description of what this example page is doing and how come we don't handle it well so far?

Also, could you explain a bit more what your proposed change involves? From your example results it seems like you're creating frame Objects for the frame's parent frame ID as well?

Is this an issue in Firefox and Edge?

comment:3 Changed 11 months ago by greiner

  • Description modified (diff)

Would you mind adding some explanation to the issue description of what this example page is doing and how come we don't handle it well so far?

I added a bit more context regarding the example page but it is not clear yet what causes the issue so there's not much I was able to add there. Considering that this is a fallback solution for any situation for which the browser doesn't notify us of a frame being created, I considered it not being necessary to find out what exactly causes it since that would be merely an optimization that could be worked on separately (i.e. the more frames we can detect early on outside of the critical path, the fewer cases there are for which we need to fall back on adding frames while on the critical path).

However, I can understand if you disagree with this assessment.

Also, could you explain a bit more what your proposed change involves? From your example results it seems like you're creating frame Objects for the frame's parent frame ID as well?

My suggested approach is only creating a frame object for the frame the request originated from, not for its parent since we don't know what its parent's parentId is. I've tried to clarify that bit now in the description.

I assume your concern is that this approach cannot deal with a situation where a request is coming from an unknown frame whose parent is also unknown. I'd consider that to be a valid argument, I just don't know how we could handle such cases considering that all we know of the parent frame is its ID.

Is this an issue in Firefox and Edge?

I haven't tested it on Edge but it also affects Firefox.

Note: See TracTickets for help on using tickets.