Closed Bug 371123 Opened 18 years ago Closed 18 years ago

XMLHttpRequest should trigger content policies

Categories

(Core :: XML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha4

People

(Reporter: ecfbugzilla, Assigned: ecfbugzilla)

References

Details

(Keywords: verified1.8.1.4)

Attachments

(2 files)

Currently XMLHttpRequest allows web sites to make a request that cannot be stopped with content policies. This can be used for tracking or for loading ads without giving users a chance to interfere with content blockers (actually, there are already sites that do this).

To test this:
1. Install Adblock Plus from https://addons.mozilla.org/firefox/1865/
2. Restart your browser and press Ctrl+Shift+B to open the "list of blockable items" (all addresses sent through content policies for the current page)
3. Go to http://jibbering.com/2002/4/httprequest.html and click the first "Try the example" link. An alert box with the contents of http://jibbering.com/2002/4/test.txt will appear.

Observed results:
"List of blockable items" doesn't show http://jibbering.com/2002/4/test.txt even though the contents of this file have been loaded.

Expected results:
http://jibbering.com/2002/4/test.txt should appear in the list as it happens for every other request.
Attached patch Proposed patch (deleted) β€” β€” Splinter Review
This fixes the bug - URLs requested with XMLHttpRequest appear in the list in Adblock Plus and can be blocked.
Assignee: xml → trev
Status: NEW → ASSIGNED
Attachment #255894 - Flags: superreview?
Attachment #255894 - Flags: review?(bzbarsky)
Attachment #255894 - Flags: superreview? → superreview?(hjtoi-bugzilla)
I'm not going to be able to review this in the foreseeable future.  Try someone else, please?
Attachment #255894 - Flags: review?(bzbarsky) → review?(jst)
Comment on attachment 255894 [details] [diff] [review]
Proposed patch

+  rv = NS_CheckContentLoadPolicy(nsIContentPolicy::TYPE_OTHER,
+                                uri,
...

Nit of the day: Second line (and following) argument indentation is one short.

r=jst

P.S. If Heikki doesn't sr this (not sure how much he keeps track of his sr requests any more), jonas@sicking.cc probably can...
Attachment #255894 - Flags: review?(jst) → review+
Comment on attachment 255894 [details] [diff] [review]
Proposed patch

I haven't been a super-reviewer for several years, so somebody else should take this. Per jst's advice asking Jonas.
Attachment #255894 - Flags: superreview?(hjtoi-bugzilla) → superreview?(jonas)
Attachment #255894 - Flags: superreview?(jonas) → superreview+
Whiteboard: [checkin needed]
Checked in with the requested whitespace change. Note that it's easier to check in patches that are diffed from /mozilla and incorporate fixes for all of the review comments.

mozilla/content/base/src/nsXMLHttpRequest.cpp  1.175
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha4
Comment on attachment 255894 [details] [diff] [review]
Proposed patch

The patch itself is pretty straightforward, no possible failure points AFAICT - it is just the content policy call as used everywhere else. However, strictly speaking this is an interface change, we trigger content policies where we didn't do it before. I still don't expect any content policies to fail since almost all of them ignore TYPE_OTHER, and the ones that don't have to handle calls for XBL requests already which have the same parameters. Advantage of taking this change on the branch: we will close this loophole now, before it becomes a popular trick to get around content policies.

If this is approved for the branch I will request approval in bug 375435 as well - basically the same reasoning.
Attachment #255894 - Flags: approval1.8.1.4?
Comment on attachment 255894 [details] [diff] [review]
Proposed patch

approved for 1.8.1.4, a=dveditz for release-drivers
Attachment #255894 - Flags: approval1.8.1.4? → approval1.8.1.4+
Whiteboard: [checking needed - 1.8 branch]
Attached patch Patch for 1.8 branch (deleted) β€” β€” Splinter Review
Whiteboard: [checking needed - 1.8 branch] → [checkin needed - 1.8 branch]
(In reply to comment #8)
> Created an attachment (id=261088) [details]
> Patch for 1.8 branch
> 

I am using Gran Paradiso 3.0a3.

A serious bug appeared in this release that prevents Unicode characters from being properly transferred via XMLHttpRequest. I think it may be related to the introduction of content filtering for extensions such as AdBlock Plus. The unicode transfer fails to properly with or without AdBlock Plus installed.

I cannot get FireBug 1.04 to work properly with 3.0a3 and XMLHttpRequests, so I cannot yet explicitly identify the problem area. Receiving unicode via XMLHttp works in prior versions of Gran Paradiso. My data contains Unicode and ASCII. The ASCII comes across fine while the Unicode simply disappears within the same one record.

Please advise as to the proper forum that this should be posted.

Thank you
This patch landed after Gran Paradiso Alpha 3 release, so it cannot have anything to do with your problem. Please create a new bug for it and include the exact steps to reproduce.
landed on 1.8 branch
Whiteboard: [checkin needed - 1.8 branch]
Keywords: fixed1.8.1.4
verified fixed 1.8.1.4 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.4pre) Gecko/2007042805 BonEcho/2.0.0.4pre and the steps to reproduce from  comment #0.

http://jibbering.com/2002/4/test.txt is now in the List of blockable items -> adding verified keyword.
Blocks: abp
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: