Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GM_xmlhttpRequest bug when adding function to Object.prototype #1065

Closed
chris0385 opened this issue Dec 9, 2009 · 6 comments
Closed

GM_xmlhttpRequest bug when adding function to Object.prototype #1065

chris0385 opened this issue Dec 9, 2009 · 6 comments
Milestone

Comments

@chris0385
Copy link

If you add a function to Object.prototype, GM_xmlhttpRequest doesn't work any more.

So the script below don't work and throw : "Erreur : uncaught exception: unknown (can't convert to string)"

// ----------------------------------------------------------------

GM_xmlhttpRequest({
method: 'GET',
url: 'http://greaseblog.blogspot.com/atom.xml',
headers: {
'User-agent': 'Mozilla/4.0 (compatible) Greasemonkey',
'Accept': 'application/atom+xml,application/xml,text/xml',
},
onload: function(responseDetails) {
alert('Request for Atom feed returned ' + responseDetails.status +
' ' + responseDetails.statusText + '\n\n' +
'Feed data:\n' + responseDetails.responseText);
}
});

/**
Erreur : uncaught exception: unknown (can't convert to string)

That was thrown while running the GM_xmlhttpRequest just above.
*/
Object.prototype.anyFunctionName=function(str) { } // BUG

//Object.prototype.x=1; // variables work

// ----------------------------------------------------------------

I Use :
Greasemonkey version : 0.8.20091209.4
Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9.0.15) Gecko/2009102814 Ubuntu/8.04 (hardy) Firefox/3.0.15

@arantius
Copy link
Collaborator

Cannot reproduce on GM 0.8.20100211.5

Test script:

Object.prototype.AnyNameHere = function(x) {}

GM_xmlhttpRequest({
    method: "GET",
    url: "http://www.example.net/",
    onload: function(responseDetails) {
      alert('Request returned ' + responseDetails.status +
            ' ' + responseDetails.statusText + '\n\n' +
            'Data:\n' + responseDetails.responseText);
    }
});

Works as expected.

@erikvold
Copy link
Contributor

erikvold commented Mar 5, 2010

arantius: It might have to do with the object literal that is used in the example chris0385 provided. I assume that the object literal is parsed for it's properties and all the properties are used in the header; so the error must occur because one property is a function (ie: a method).

If this is the case then one way to correct this bug would be to test that the property is not a method before attempting to use it.

@erikvold
Copy link
Contributor

erikvold commented Mar 5, 2010

the headers object literal that is..

@Ventero
Copy link
Contributor

Ventero commented Mar 9, 2010

Yeah, I can confirm that the function-property of the headers-object is causing this problem. The relevant code is L74-L78 in content/xmlhttprequest.js.

As erikvold said, a possible fix is to check if the property is a function.

Additionally it might be a good idea to throw a

if(details.headers.hasOwnProperty(prop))

in there.

Edit: Actually not functions itself cause the problem, but multi-line strings.
For example,

Object.prototype.foo = "foo\nbar;
GM_xmlhttpRequest({method: 'GET', url: 'http://github.com/', headers: {}});
throws the same exception (which is
"Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXMLHttpRequest.setRequestHeader]" for reference).
The reason why it throws the same error for functions is, that a function's toString() always returns a multi-line string. So checking the typeof for each property definitely isn't enough.

@erikvold
Copy link
Contributor

Nice work Ventero!

I took a look at yui and using the hasOwnProperty() method seems to be what they use as well. I also did some reading on it and it does appear to be the correct method to use.

So I created a branch from the greasemonkey/master branch called issue-1065 and made the change here

I also tested this change with this userscript and it works.

@arantius
Copy link
Collaborator

arantius commented Apr 5, 2010

commit 6000de1

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants