Opened 2 years ago

Last modified 2 years ago

#6565 closed defect

Cannot destructure property `protocol` of 'undefined' exceptions in background console — at Version 6

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

https://codereview.adblockplus.org/29750566/
https://codereview.adblockplus.org/29753574/

Description

Environment

Chrome 65, Adblock Plus devbuild built from 118c7856d36d.

How to reproduce

  1. Open the background console for the extension.
  2. Open a new tab and browse to https://reddit.com

Observed behaviour

This exception shows in the background console multiple times a tab is opened.

Error in event handler for webRequest.onBeforeRequest/28: TypeError: Cannot destructure property `protocol` of 'undefined' or 'null'.
    at exports.stringifyURL.url (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:2229:26)
    at match (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:3910:19)
    at exports.checkWhitelisted (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:3958:18)
    at browser.webRequest.onBeforeRequest.addListener.details (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:6662:7)

Expected behaviour

No exceptions showing in the background console.

Notes

Change History (6)

comment:1 follow-up: Changed 2 years ago by sebastian

I cannot reproduce this. I tested on Chrome 63 and 65. Also from just looking at the code, I cannot see how these changes will cause checkWhitelisted() to call match() which then calls stringifyURL() with undefined as url (if it didn't before). I might miss something, but without being able to reproduce the error, I cannot track it down.

comment:2 in reply to: ↑ 1 Changed 2 years ago by kzar

Replying to sebastian:

I cannot reproduce this. I tested on Chrome 63 and 65.

Weird, since I can reproduce this 100% reliably.

Also from just looking at the code, I cannot see how these changes will cause checkWhitelisted() to call match() which then calls stringifyURL() with undefined as url (if it didn't before). I might miss something, but without being able to reproduce the error, I cannot track it down.

From running this through the debugger it *seems* like it could be since previously the onBeforeRequest listener in lib/requestBlocker.js only called checkWhitelisted if frame was truthy, but now it always does. In any case I see that page.url is undefined, but details.url isn't. Perhaps we should modify the line in onBeforeRequest where we create the ext.Page object to pass in the URL?

Last edited 2 years ago by kzar (previous) (diff)

comment:3 Changed 2 years ago by sebastian

Sounds good. I could write a patch, but since you would have to test the change anyway (as I cannot reproduce the issue), you might want to go ahead. Otherwise let me know.

comment:4 Changed 2 years ago by kzar

  • Owner set to kzar

Sure OK no problem, I'll get a patch together.

comment:5 Changed 2 years ago by kzar

I see now it's the New Tab page that makes requests for each of the suggested pages so that it can show a preview of them (and I guess so they're preloaded ready).

comment:6 Changed 2 years ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing
Note: See TracTickets for help on using tickets.