Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

GM_xmlhttpRequest + POST #1032

Closed
avindra opened this Issue Sep 20, 2009 · 11 comments

Comments

Projects
None yet
5 participants

avindra commented Sep 20, 2009

I think you should check for "POST" requests and automatically set the "Content-Type" header to "application/x-www-form-urlencoded" if that's the case. I have yet to bump into a POST request which doesn't require this header to be set, and it'd save writers from having to type:

headers : {
"Content-Type" : "application/x-www-form-urlencoded"
}

every time.

possible patch:

   if (details.headers) {
     if(!details.headers["Content-Type"] && req.method.toUpperCase()=="POST")
       req.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
     for (var prop in details.headers)
       req.setRequestHeader(prop, details.headers[prop]);
   }

Patch would look something like this

--- content/xmlhttprequester.js (revision HEAD)
+++ content/xmlhttprequester.js Mon Sep 21 00:35:34 CEST 2009
@@ -69,11 +69,17 @@
     req.overrideMimeType(details.overrideMimeType);
   }

+  var contentTypeSet = false;
   if (details.headers) {
     for (var prop in details.headers) {
       req.setRequestHeader(prop, details.headers[prop]);
+      if (prop.toLowerCase() == "content-type") contentTypeSet = true;
     }
   }
+  // If request method is POST, the content type needs to be set for it to work as expected
+  if (details.method.toUpperCase() == "POST" && !contentTypeSet) {
+    req.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
+  }

   req.send((details.data) ? details.data : null);
   GM_log("< GM_xmlhttpRequest.chromeStartRequest");

avindra commented Sep 21, 2009

@Photodeus:

why not:

   if (details.headers) {
     if(!details.headers["Content-Type"] && req.method.toUpperCase()=="POST")
       req.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
     for (var prop in details.headers)
       req.setRequestHeader(prop, details.headers[prop]);
   }

avindra commented Sep 21, 2009

Actually, something along those lines, but outside of

if (details.headers) {

Because usually, that may be the only header you'd need to set anyway.

Need the prop.toLowerCase() == "content-type" check, cause you can't be sure script author uses exact same capitalization.

Contributor

Martii commented Sep 21, 2009

Couple of things from my experience and the W3/IANA/etc... Content-Type should be cased this way as that's the standard. Also it should never be defaulted to application/x-www-form-urlencoded. This is not always used in every scenario. Having the ScriptWright specify it manually will cut down on the number of mistakes by making them actually look up what a site is expecting normally.

-1

Header fields are case insensitive, so requiring any kind of type casing would be a stricter requirement than the RFC itself.
http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

HTTP header fields, which include general-header (section 4.5), request-header (section 5.3), response-header (section 6.2), and entity-header (section 7.1) fields, follow the same generic format as that given in Section 3.1 of RFC 822 [9]. Each header field consists of a name followed by a colon (":") and the field value. Field names are case-insensitive.

I'm not sure about any unwanted side-effects if the POST data is by default set to something that works in the majority of cases. I'm still reading references and making tests.
Per HTML 4 spec
http://www.w3.org/TR/1999/REC-html401-19991224/interact/forms.html#h-17.13.4

application/x-www-form-urlencoded

  • This is the default content type.

So in any case, if scriptwright has forgotten to add any type of Content-Type, Greasemonkey needs to either set the value or complain about it being absent.

Also the specification states:

The content type "application/x-www-form-urlencoded" is inefficient for sending large quantities of binary data or text containing non-ASCII characters. The content type "multipart/form-data" should be used for submitting forms that contain files, non-ASCII data, and binary data.

If we add binary support for GM_xmlhttpRequest, it could/should warn if scriptwright tries to use a Content-Type inappropriate for the type of request.

Another reference for completeness sake:
http://en.wikipedia.org/wiki/Percent-encoding#The_application.2Fx-www-form-urlencoded_type

Contributor

Martii commented Sep 21, 2009

@Photodeus

Looks like a few things may have changed at W3 since I last investigated this... however I usually pay attention to Apache and IIS to see what their defaults are... those are still Content-Type and no default type. Part of my policy is making GM completely undetectable to a server and defaulting to something that is GM specific could compromise a scripts execution.

A good question to ask is if someone uses content-type and GM defaults to Content-Type... Will there be two entries on ALL http daemons and browsers out there?

Also pulling from the current doc with full citation

This is the default content type. Forms submitted with this content type must be encoded as follows:
Not everything follows a form submittal criteria and another question would be... How would one go about "erasing" an existing Content-Type if none is required? Again this leads back to full transparency of GM.

Reval to ± 0 since there are some discrepancies.

Closed by johan

Collaborator

arantius commented Oct 6, 2009

I vote -1. The benefit is minimal (saving one line of code?) and it's a perfectly normal JS/AJAX thing that a programmer needs to know.

Agreed then. As long as it's properly documented in the GM_xmlhttpRequest documentation, then I think the "needs to know" criteria is fulfilled.

Collaborator

johan commented Oct 9, 2009

A final -1 and close. Making browser APIs comfier is the domain of @require libraries.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment