669555 - Diff-Viewer: Fragment URIs should not contain the plain text file path and name

Status: RESOLVED WONTFIX

(addons.mozilla.org Graveyard :: Admin/Editor Tools, defect, P4)

Description

[Nils Maier [:nmaier]]

Currently the diff-viewer makes XHR requests like this:
GET /en-US/firefox/files/browse/122983/fragment/chrome.manifest HTTP/1.1

However, having the file path and name as part of the fragment URI might cause interference from various tools, such as extensions implementing nsIContentPolicy, e.g. Adblock Plus or Greasemonkey/Scriptish, or https-proxies implementing some kind of blacklist (security appliances).

It hence would be better to add the requested fragment file path to the request using some other method, such as another custom header, a post request or whatever

Comment 1

[Jeff Balogh (:jbalogh)]

You named possible pain points, but is there something in particular that's actually blocking you?

Comment 2

[Jorge Villalobos [:jorgev] (he/him)]

This bug came up in a discussion in the editors internal list, because Scriptish blocks *.user.js scripts in the file diff viewer. It shouldn't be too difficult to work around it.

Comment 3

[Jeff Balogh (:jbalogh)]

It sounds like a bug on their side, not on ours. Putting the filename in the URL is the only thing that makes sense to me. Otherwise we become a busted PHP site from the 90s.

Comment 4

[Andy McKay]

+1 on trying to keep the full file path in the url if possible.

Comment 5

[Jorge Villalobos [:jorgev] (he/him)]

Yeah, I'm not convinced either. At least for ABP and NoScript there's a way to manage the whitelist / blacklist. I don't know what the case is for GM or Scriptish. Erik should have a better idea about this.

Comment 6

[Kris Maglione [:kmag]]

I also agree that this is a Scriptish/Greasemonkey bug, nor do I see why they should be blocking XMLHttpRequests at all. On the other hand, as they're extremely popular extensions, I think this needs to be dealt with.

Comment 7

[Erik Vold]

(In reply to comment #6)
> I also agree that this is a Scriptish/Greasemonkey bug, nor do I see why
> they should be blocking XMLHttpRequests at all.

I agree with this. Maybe there is something we can do?

https://github.com/erikvold/scriptish/blob/master/extension/components/scriptish.js#L268

I'll try to look into it.

Comment 8

[Erik Vold]

It looks like we can use the TYPE_XMLHTTPREQUEST to ignore xhr

https://developer.mozilla.org/en/nsIContentPolicy

Comment 9

[Erik Vold]

Scriptish only seems to reject *.user.js for TYPE_DOCUMENT and TYPE_SUBDOCUMENT

https://github.com/erikvold/scriptish/blob/master/extension/components/scriptish.js#L285

so I'm confused why it's rejecting a xhr.. I'll test it out later tho.

Comment 10

[Erik Vold]

@Kris is there a extension in the queue that has user.js files that I can test?

Comment 11

[Kris Maglione [:kmag]]

This should suffice: https://addons.mozilla.org/en-US/firefox/files/browse/124285/

None of the files in the userscripts directory are accessible.

Comment 12

[Nils Maier [:nmaier]]

Forgot about TYPE_XMLHTTPREQUEST. Scriptish v0.1 and master both do not block the .user.js files for me.

Kris, can you verify Scriptish is to blame and if it is file a Scriptish issue according to: http://goo.gl/RTsDR

The consensus here seems to be that this bug is INVALID... I would suggest resolving it as such.

Comment 13

[Jorge Villalobos [:jorgev] (he/him)]

Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...