Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#1562 closed change (fixed)

Improve detecting of XmlHttpRequest

Reported by: sergz Assignee: sergz
Priority: P2 Milestone: Adblock-Plus-for-Internet-Explorer-1.3
Module: Adblock-Plus-for-Internet-Explorer Keywords:
Cc: oleksandr, arthur Blocked By:
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/5634261554036736/

Description (last modified by sergz)

Background

For requests issued with XMLHttpRequest IE sets header X-Requested-With: XMLHttpRequest. As well as we can classify requests with Origin header as xmlhttprequests.

What to change

Add the required functionality into PluginWbPassThrough.cpp

Change History (6)

comment:1 Changed 5 years ago by sergz

  • Status changed from new to reviewing

comment:2 Changed 5 years ago by sergz

  • Description modified (diff)

comment:3 Changed 5 years ago by sergz

  • Cc arthur added

@oleksandr said:

added CORS

I think for the time being at least there is more harm in not classifying some XMLHttpRequests as XMLHttpRequest, then in classifying some WebGL or WebFonts requests accidentally as XMLHttpRequest. I'd vote for pushing this.


I would drop detecting request with Origin header as XMLHttpRequest.

  • I've checked my patterns.ini and there are only several blocking rules which contain mere 'xmlhttprequest' and nothing else like 'script', 'third-party', so I would say it should not harm a lot.
  • I would prefer to adjust the filter instead if something is not blocked. Because, if we want to block some resource we should not rely on the querying mechanism, whether it's loaded via some tag, some css construction, using javascript objects like XMLHttpRequest or even some extension like Flash addon is trying to load it. I would like the filter makers to concentrate on the nature of the resource, pay attention to its type and location.
  • Instead of classifying request with Origin header as XMLHttpRequest, we should rather classify it as 'third-party' request.

So, I would propose to drop it and see whether it's not enough with such approach.

comment:4 Changed 5 years ago by sergz

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

Committed "patch set 1"

comment:5 Changed 5 years ago by fhd

  • Milestone set to Adblock-Plus-for-Internet-Explorer-1.3

comment:6 Changed 5 years ago by fhd

  • Priority changed from Unknown to P2
  • Ready set

Posthumously setting priority and ready - P2 seems about appropriate here.

Note: See TracTickets for help on using tickets.