-
Notifications
You must be signed in to change notification settings - Fork 180
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
got IOError during POST with Form email #825
Comments
I can reproduce on latest Julia & HTTP. The same request seems to go through on JS: ➜ ~ deno
Deno 1.19.2
exit using ctrl+d or close()
> let url = "http://post.oreilly.com/client/o/oreilly/forms/quicksignup.cgi"
undefined
> let data = new FormData()
undefined
> data.append("email_addr", "[email protected]")
undefined
> let r = await fetch(url, {
method: "post",
body: data,
})
undefined
> await r.text()
`\n<!DOCTYPE html>\n<html lang="en"><head>\n <meta charset="utf-8">\n <meta http-equiv="X-UA-Compatible" content="IE=edge">\n <meta name="viewport" content="width=device-width, initial-scale=1">\n <!-- The above 3 meta tags *must* come first in the head; any other head content must come *after* these tags -->\n <meta name="description" content="Keep ahead of what's next with the latest ideas, and knowledge you can put into action, in all the subjects you care about, from O'Reilly's community of experts.">\n <meta name="author" content><meta name="robots" content="index, nofollow">\n\n <link rel="shortcut icon" href="https://www.oreilly.com/favicon.ico" type="image/vnd.microsoft.icon">\n\n <title>O'Reilly - Subscription Center</title>\n\n <link rel="stylesheet" type="text/css" href="https://cdn.oreillystatic.com/assets/css/2017_font_face.css">\n <link rel="stylesheet" type="text/css" href="https://cdn.oreillystatic.com/assets/css/subscription-center-topics.css">\n \n <noscript>\n <style>\n .mktoForm ...... etc |
Fixes #825. TIL: not all redirects should use the same method as the original request, and in fact, most http client implementations will default the new request method to GET. I included links to a few useful discussions/references, but essentially the behavior we now support is: * For 307/308 response status, use original request method; these statuses are specifically for indicating the original method is ok * For 303 response status, must use GET method * For other 3xx responses, it's actually legal to use the original method, but most client implementations will switch to GET, which means most servers have adapted to expect this and may even error if the original request method is used (as is the case in the originally reported issue). We now follow this behavior, though, like curl, allow overriding this behavior by a new `redirect_method` keyword arg to allow specifying the redirect method behavior.
Thanks for the report @deahhh and for confirming @fonsp! I have a PR up that should fix this case: #829, and clean up our "which method should be used in redirect requests" behavior more generally. The core issue here is we were using/sending the original request method (POST) in the redirect request, which the server did not accept/support and hence the error. It turns out that should be legal in this case since the server is returning a 301 response status, but in practice, most http clients will switch POST -> GET, so most servers now expect that behavior (hence why other language implementations were working here and we were erroring). I am proposing a new |
Fixes #825. TIL: not all redirects should use the same method as the original request, and in fact, most http client implementations will default the new request method to GET. I included links to a few useful discussions/references, but essentially the behavior we now support is: * For 307/308 response status, use original request method; these statuses are specifically for indicating the original method is ok * For 303 response status, must use GET method * For other 3xx responses, it's actually legal to use the original method, but most client implementations will switch to GET, which means most servers have adapted to expect this and may even error if the original request method is used (as is the case in the originally reported issue). We now follow this behavior, though, like curl, allow overriding this behavior by a new `redirect_method` keyword arg to allow specifying the redirect method behavior.
bug report with:
output as
The text was updated successfully, but these errors were encountered: