Opened 4 months ago

Closed 3 weeks ago

Last modified 10 days ago

#4647 closed defect (fixed)

onBeforeRequest listener throwing exception for frames without a URL

Reported by: kzar Assignee: kzar
Priority: P1 Milestone: Adblock-Plus-1.13-for-Chrome-Opera
Module: Platform Keywords:
Cc: trev, sebastian, BrentM, rhana@…, arthur Blocked By:
Blocking: #4598 Platform: Chrome
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29362605/

Description

Environment

adblockpluschrome 12a8449bde06
Debian Stretch
Chrome Version 54.0.2840.59 (64-bit)

How to reproduce

  1. Open the console for the extension's background page.
  2. Browse the web for a while.

Observed behaviour

Exceptions being thrown by the onBeforeRequest handler, for frames without a URL property.

Expected behaviour

Those exceptions should not be thrown.

Notes

The onBeforeRequest listener assumes that the originating frame will always have a URL. This is no longer true however since we now create frames with only the parent property in the onBeforeNavigate handler. Therefore I think this is a regression caused by Issue 4386 - Fixed determining document domain, particularly after being redirected.

We should now either ignore requests from frames without a URL or adapt the onBeforeRequest handler so that it does not assume the frame URL will be available. (Might be worthwhile since the parent frame might already have a URL.)

Change History (8)

comment:1 Changed 4 months ago by kzar

  • Blocking 4598 added

comment:2 Changed 4 months ago by kzar

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

comment:3 Changed 4 months ago by kzar

  • Review URL(s) modified (diff)

comment:4 Changed 4 months ago by kzar

  • Cc rhana@… added

comment:5 Changed 4 months ago by arthur

  • Cc arthur added

comment:6 Changed 2 months ago by sebastian

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next

comment:7 Changed 3 weeks ago by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed

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

comment:8 Changed 10 days ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Looks fixed. Could not reproduce exception.

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

Last edited 10 days ago by Ross (previous) (diff)
Note: See TracTickets for help on using tickets.