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

Add an option to allow GET with body #1074

Closed
szmarczak opened this issue Feb 17, 2020 · 7 comments
Closed

Add an option to allow GET with body #1074

szmarczak opened this issue Feb 17, 2020 · 7 comments
Labels
enhancement This change will extend Got features ✭ help wanted ✭

Comments

@szmarczak
Copy link
Collaborator

szmarczak commented Feb 17, 2020

Currently Got throws TypeError: The GET method cannot be used with a body when you try to upload some data using the GET method. The RFC 7231 doesn't specify any particular behavior regarding this situation. Undefined behavior is considered to be an anti-pattern.

Although this has been discussed in some issues, there are many people who are still interested in this. Sometimes the HTTP servers don't follow the RFC. We have already added an option to ignore invalid cookies. The proposed solution is to disable the error via another option, for example ignoreStrictPayload allowGetBody.

@szmarczak szmarczak added enhancement This change will extend Got features ✭ help wanted ✭ labels Feb 17, 2020
@sindresorhus
Copy link
Owner

Wouldn't it be clearer with an option called allowGetBody?

@szmarczak
Copy link
Collaborator Author

Sure.

@sindresorhus
Copy link
Owner

In addition, to anyone that decides to work on this. The docs should strongly advice against using this option and explain that it's an anti-pattern and that HTTP2 doesn't allow a GET with body. That it's only meant to interact with non-compliant servers when you have no other choice.

Repository owner deleted a comment from dcharbonnier Feb 17, 2020
@dopecodez
Copy link
Contributor

Hey @sindresorhus , i just migrated from request to got and might have a edge case where I may have to support a body on a GET request. I was looking through the code and it would like to contribute a PR to fix this issue with the option allowGetBody as discussed. Could i take this up if no-one is working on this right now?
Also looking through the code, i noticed we don't support body on both GET and HEAD requests. Since the RFC 7321 isn't clear on whether we should support a body on a HEAD request, should we think about supporting this as well? (Although i can't imagine a use case where a HEAD request could have a body). Thanks.

@szmarczak
Copy link
Collaborator Author

Could i take this up if no-one is working on this right now?

Sure. Feel free.

Since the RFC 7321 isn't clear on whether we should support a body on a HEAD request, should we think about supporting this as well?

Personally I think that we shouldn't do that for HEAD, as it's used only for scraping headers. If there would be any use case, then it should be another issue.

@dopecodez
Copy link
Contributor

Should be fixed with PR #1081

@dopecodez
Copy link
Contributor

I think this issue can be closed @sindresorhus @szmarczak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This change will extend Got features ✭ help wanted ✭
Projects
None yet
Development

No branches or pull requests

3 participants