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

GET operations cannot have a request #546

Closed
dannysheridan opened this issue Aug 8, 2022 · 10 comments · Fixed by #843
Closed

GET operations cannot have a request #546

dannysheridan opened this issue Aug 8, 2022 · 10 comments · Fixed by #843
Assignees

Comments

@dannysheridan
Copy link
Member

Semantic error at the path (e.g. "paths./v2/link/provider/oauth/{oauth_provider}.get.requestBody")
GET operations cannot have a requestBody.

@dannysheridan
Copy link
Member Author

Screen Shot 2022-08-08 at 3 56 50 PM

@dannysheridan
Copy link
Member Author

caught by swagger editor!

@dsinghvi
Copy link
Member

dsinghvi commented Aug 9, 2022

swagger-api/swagger-ui#2136 check out this issue, i don't think we should actually validate against this

@dsinghvi
Copy link
Member

dsinghvi commented Aug 9, 2022

OAI/OpenAPI-Specification#2117 maybe we can warn instead

@zachkirsch
Copy link
Contributor

From MDN:

Note: Sending body/payload in a GET request may cause some existing implementations to reject the request — while not prohibited by the specification, the semantics are undefined. It is better to just avoid sending payloads in GET requests.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/GET

@zachkirsch
Copy link
Contributor

Happy to disallow it like conjure?

@dannysheridan
Copy link
Member Author

I'm in favor of warning but allowing.

@zachkirsch
Copy link
Contributor

Is there a use case where someone needs it? If certain server implementations don’t allow get bodies (per MDN) this could cause issues. Also a good place to encode an opinion.

@dannysheridan
Copy link
Member Author

+1 to warn against having a Request in a GET body.

@dannysheridan
Copy link
Member Author

+1 to this.

dsinghvi added a commit that referenced this issue Feb 11, 2024
* use axios-retry for retry logic

* update sdk snapshots

* pass in max retries

* pass in max retries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants