Updates redirect handling #76
Conversation
|
I am having this same issue and this pull request fixes it, please merge. |
|
I would really like to see this merged. |
|
Also getting errors on a 302 response, but getting: However, the connection is definitely not refused as the server logger registers the request. |
| this.url = url; | ||
| if(res.statusCode !== 303 && this._redirectData && this.method !== 'HEAD' && this.method !== 'GET' ){ |
There was a problem hiding this comment.
this line is really long, i'd rather have something like var seeOther = 303 == res.statusCode;\n var idempotent = 'HEAD' == this.method || 'GET' == this.method etc then followed by conditional. Also add a space if (... like the rest of the source
|
needs a rebase but other than the style related stuff I'll merge, thanks! |
|
I think I did what I needed to...not sure about the rebasing |
|
just realized you dont have any tests for this new code |
added tests for redirect with data and authorization
fixed variable name
|
Added tests. Thank you for having me do this. It helped me find a few issues and taught me a lot. |
changed to send(this_data) rather than just setting the content type, just in case.
| request | ||
| .post('http://localhost:3003/addmovie') | ||
| .send({ name: 'Methos' }) | ||
| .redirectData(true) |
There was a problem hiding this comment.
just noticed this, we shouldn't ever do this I dont wan't to buffer data and this will not support streams etc
There was a problem hiding this comment.
and this patch should have been more focused, this is a completely separate thing, I just didn't read very well :D
When doing a post and getting a redirect response superagent tries to do another post and errors as there is no data to write.
error is "first argument must be a string or buffer"
also tries to redirect using same method when this is forbidden according to the spec
(http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3) without user confirmation
tested against http://jigsaw.w3.org/HTTP/300/