643156 - No mechanism to perform cross-domain HTTP HEAD request with add-on builder

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, enhancement, P3)

Description

[David Bruant]

The problem I am pointing is related to https://builder.addons.mozilla.org/ (sorry if I have mischosen the Component).
_My_use_case:_
The add-on I wanted to write is a linkchecker. Basically, in a page, with one right click in contextual menu, someone can check if all links within a page are valid (200) or broken (>=400) and highlight them. I could do it with a GET request. However, a GET request downloads the request body which is a shame since I only want the status code. For that reason, there is a HEAD HTTP request which, as per the HTTP spec behaves like GET, but do not return the body.
I wanted to use a HEAD request in the add-on builder, but with the request helper, only GET and POST can be performed. Moreover, with xhr, I cannot go cross-domain (which prevents from checking cross-domain links).
What I am asking in this bug is a cross-domain HEAD HTTP request.

Comment 1

[Boris Zbarsky [:bzbarsky]]

For what it's worth, there is a built-in component that does this already.  See nsIURIChecker.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Also note that this built-in component works around some known server bugs with HEAD, etc....

Comment 3

[David Bruant]

(In reply to comment #1 & comment #2)
> For what it's worth, there is a built-in component that does this already.  See
> nsIURIChecker.

That sounds like exactly what I want. I'd be happy to know more.
However, I haven't been able to find anything about that on https://builder.addons.mozilla.org/api/
Neither on MDN. There is the interfaces page (https://developer.mozilla.org/en/Interfaces) where nsIURIChecker is listed, but the page isn't written.
Would there be another page to look up to know how to use it at all and within the addon-builder?


> Also note that this built-in component works around some known server bugs with
> HEAD, etc....
I have a bit of experience with this kind of linkchecker problem and crawling etc. and I am aware that there are some bug servers indeed. If there is a component able to deal with them without having to re-discover and re-code the tricks, then I am even more excited to learn more about it :-)

Comment 4

[Boris Zbarsky [:bzbarsky]]

http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsIURIChecker.idl describes the API.  I have no idea how to use it within the addon builder, but http://mxr.mozilla.org/comm-central/source/editor/ui/dialogs/content/EdLinkChecker.js is the code the Seamonkey suite's editor uses to check links.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Adding HEAD support to the Request API seems like a perfectly reasonable request.

In the meantime, however, and since it's a better solution anyway for this particular problem, the way to use nsIURIChecker in an SDK-based addon is to get a reference to the Components object via the special "chrome" pseudo-module, use that object to access nsIURIChecker in your main addon script, and use a content script to interact with the content of the page for which your context menu item was clicked, i.e. something like this:

var contextMenu = require("context-menu");
var { Components } = require("chrome");

var myItem = contextMenu.Item({
  label: "Check Links",
  context: contextMenu.PageContext(),
  contentScript: 'on("click", function () {' +
                 '  /* Send the main addon script an array of links. */' +
                 '  postMessage([a.getAttribute("href") for each (a in document.getElementsByTagName("a")]);' +
                 '});' +
                 'on("message", function (brokenLinks) {' +
                 '  /* do something to the links that are broken */' +
                 '});',
  onMessage: function (links) {
    links.forEach(function() {
      /* use the Components object to access nsIURIChecker the same way
         Seamonkey does, then use myItem.postMessage to send the content
         script an array of broken links. */
    });
  }
});

Caveat #1: require("chrome") is an internal API that is subject to change in the future.

Caveat #2: postMessage can only pass JSONable information between the main addon script and the content script.  Arrays of links work if the links are represented as strings, but you wouldn't be able to pass the DOM nodes themselves.

Caveat #3: I don't actually know how nsIURIChecker works, so I can't provide insight into the implementation of onMessage in the main addon script.

Caveat #4: I haven't actually tested this code, so I'm not sure whether or not it works.

Also note that content scripts can be easier to develop if you put them in separate files.  To do so, put the script in a file within your addon's data/ directory and use the contentScriptFile property instead of the contentScript property to reference it, assigning that property a reference to the content script that you obtain from the `self` module:

  contentScriptFile: require("self").data.url("my-content-script.js")

Comment 6

[David Bruant]

(In reply to comment #5)
> Adding HEAD support to the Request API seems like a perfectly reasonable
> request.
Thanks :-) Out of curiosity, what is the track you're planning on taking?
I'm a bit surprised by the 'request' API. XMLHttpRequest is an already existing "complete" (for regular web dev cases) JavaScript abstraction of HTTP requests. Wouldn't it be easier to just "unlock" the cross-domain barrier when used within an add-on?
XMLHttpRequest is already known by all web devs (who is the add-on builder target if I'm not mistaken), so they don't need to learn a new API.


> In the meantime, however, and since it's a better solution anyway for this
> particular problem, the way to use nsIURIChecker in an SDK-based addon is to
> get a reference to the Components object via the special "chrome"
> pseudo-module, use that object to access nsIURIChecker in your main addon
> script, and use a content script to interact with the content of the page for
> which your context menu item was clicked, i.e. something like this:
> (...)
Thank you soooooo much!!
I'll find some time next week to work on my add-on based on your code.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #6)
> Out of curiosity, what is the track you're planning on taking?
> I'm a bit surprised by the 'request' API. XMLHttpRequest is an already existing
> "complete" (for regular web dev cases) JavaScript abstraction of HTTP requests.
> Wouldn't it be easier to just "unlock" the cross-domain barrier when used
> within an add-on?
> XMLHttpRequest is already known by all web devs (who is the add-on builder
> target if I'm not mistaken), so they don't need to learn a new API.

Our goal with Request is to provide a simpler, higher-level API like the ones in JavaScript libraries such as jQuery.

However, you're right that web developers are already familiar with XMLHttpRequest, and there's value in providing it to them.

Currently, it is available via the low-level "xhr" module in the api-utils package, although we don't recommend that addon developers use those modules, as they are subject to change.

We've talked about creating a high-level "web" module that gives access to XMLHttpRequest, setTimeout (and friends), and other globals that are typically available to web pages, since there are a number of such APIs with which web developers are already familiar, but we haven't done so yet.

We've also talked about loading addon code in a web page context that has access to web page globals by default, which is what Google Chrome does.  Such contexts are more heavyweight than the minimal JavaScript contexts we currently use, however, and we're concerned about the potential cost of creating one for each addon, especially when we move addon execution out of process and create a separate process for each one.

Comment 8

[Boris Zbarsky [:bzbarsky]]

One more thing worth thinking about:  HEAD responses on many servers are broken.  The best-case scenario is that they send the response body along with the headers (pretty common).  A common worst-case scenario is that the returned headers look nothing like what GET returns.

We used to try to use HEAD in our "save link as" code.  We had to give up because it was so broken.

So if we do expose it, it may be worth cautioning people that this really doesn't do what they would like to think it does based on the HTTP spec.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(Pushing all open bugs to the --- milestone for the new triage system)

Comment 10

[John Nagle]

I too have a use case for this feature: checking ad links to see where the redirects actually end up.

I'd like to have a ".head()" method on the Request object, and a ".url()" method on the Response object, to report what URL eventually was delivered after redirection.  Currently, there's no way via Request to get the URL. 

Others have asked that the Request method allow adding request headers. (See "https://forums.mozilla.org/addons/viewtopic.php?p=13309") It's time to finish the Request API and provide the full functionality one finds in most HTTP
request implementations, as in Python.

Using Chrome APIs in Jetpack add-ons will cause add-ons to fail the AMO validator. Mixing the two causes potential problems between changes in Mozilla, changes in Jetpack, and the automatic Jetpack add-on rebuilding and update process.  So the Jetpack add-on APIs need to be more complete.

Comment 11

[stephane]

This might be off-topic, but following-up on the previous comment about "finish[ing] the Request API", adding REST methods (not only HEAD, but DELETE, COPY -- my particular interest is with CouchDB) would be great. Following up in the steps of https://github.com/mikeal/request , this could be done with additional methods, or a `method` option.

I'll try to start with DELETE in my own copy of the SDK, to see whether this is as easy as it seems.

Comment 12

[stephane]

Attachment: Changes to add a .delete method to send a DELETE request.

Comment 13

[Alexandre Poirot [:ochameau]]

Comment on attachment 704852 [details] [diff] [review]
Changes to add a .delete method to send a DELETE request.

Review of attachment 704852 [details] [diff] [review]:
-----------------------------------------------------------------

Stephane, Could you factorize the condition instead of repeating it twice?
Something like this:
let passDataInURI = (mode != "POST" && mode != "PUT");
...
if (data && passDataInURI)
  url = ... + data;
...
xhr.send(data && !passDataInURI ? data : null);
...
It would ensure passing the data either in URI or in send() even if we do a mistake.

Then it would be really cool to have test for that, but I can do that if you don't have time.
Here is an example: https://github.com/mozilla/addon-sdk/blob/master/test/test-request.js#L67-L81

Thanks for your contribution!

Comment 15

[stephane]

Attachment: Add a .delete method to the request module.

This version includes reviewer comments.

Comment 16

[stephane]

I'll try to see whether I can write a test using the example for testComplexHeader.

Comment 17

[stephane]

Attachment: Test case for the .delete method of the request module

I do not know how to run the test suite, so make no claim about whether this might even work!

Patch is against 1.1.2, not against trunk.

Comment 18

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 1004

Submitting Brandon's pull request to put the review stamp.

Comment 19

[Alexandre Poirot [:ochameau]]

Comment on attachment 706063 [details] [diff] [review]
Test case for the .delete method of the request module

Stephane, I think you are almost here. Unfortunately, the patch will be bitrotten by the new patch that introduce HEAD request.
You do not need to do this deleteRequestHandler.toString()/prepareFile.
Look at what has been done for HEAD request here:
  https://github.com/mozilla/addon-sdk/pull/1004
It should be similar.

I would suggest you to open another bug to add DELETE method.
(And do not forget to ask for review, otherwise it is very easy to miss your patch)

Comment 20

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/cedd8fb617b618443694f004a971344acfcf66b7
Merge pull request #1004 from bkase/bug643156

Bug 643156 - Added support for HEAD requests r=@ochameau