Opened on 11/13/2014 at 09:14:15 AM

Closed on 11/21/2014 at 11:14:30 AM

Last modified on 12/12/2014 at 09:38:36 AM

#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

Attachments (0)

Change History (6)

comment:1 Changed on 11/13/2014 at 09:14:50 AM by sergz

  • Status changed from new to reviewing

comment:2 Changed on 11/13/2014 at 10:46:05 AM by sergz

  • Description modified (diff)

comment:3 Changed on 11/21/2014 at 10:54:41 AM 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 on 11/21/2014 at 11:14:30 AM by sergz

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

Committed "patch set 1"

comment:5 Changed on 12/12/2014 at 09:33:59 AM by fhd

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

comment:6 Changed on 12/12/2014 at 09:38:36 AM by fhd

  • Priority changed from Unknown to P2
  • Ready set

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

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from sergz.
 
Note: See TracTickets for help on using tickets.