-
Notifications
You must be signed in to change notification settings - Fork 9k
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 method doesn't support body payload #2136
Comments
Body payloads are not supported by swagger in a GET operation, and is typically not supported by any server frameworks, even though the http specification is vague about supporting them. |
I think it's a good idea. The same principle works elastichsearch. This solution is convenient when you need to maintain the concept of REST API (read, create, update, and delete), and for the GET request is necessary to pass a sufficiently large amount of data. Yes, it is not as specified, but you are developing a tool, how to use it already solves the developer. Trimming functionality you force developers to do more possible bad decisions or make patches on your project. |
+1 The HTTP spec does not forbid using body on a GET. De facto, a very popular server product (ElasticSearch) does use the body on a GET. Regardless of previous convention, the scenario should be supported. This is fallout from the move from "POST is for form submissions, GET is for page requests" way of thinking to "JavaScript calls RESTful APIs through AJAX and updates the DOM" way of thinking. Logically, a request can be a GET (I am retrieving an entity) while the specification of the entity to retrieve can be too complex to put in the path or url parameters and so the right place to put it is in the body. (ie a JSON object) |
Read the HTTP spec. It specifically says that a body in a GET is not forbidden, however the server must ignore the value. We're not adding support for that in the tool in the short term. |
Perhaps you are looking at an older version of the spec? In the current spec:
The sender's allowed to send a body in a GET.
I don't see any mandate to disregard the body of a GET. |
I ran into the same issue, trying to send a body to a GET against Elasticsearch (which it supports). I solved the issue by using a POST instead and configuring Swagger that way. |
Is there a possible compromise on this point? I work on internal API's where GET bodies are particularly useful to implement complex semantic search over a collection of tree-like objects. The real issue with GET bodies is the behavior of caching proxies. Caching proxies do not generally allow GET bodies as an optimization. But I do not think API design should be limited by the implementation details of caching proxies. It may be however that the UI should not offer this unless the swagger spec specifically identifies body characteristics? |
@jconti tbh, I don't see that happening. In 3.0, we have explicitly expressed that payloads with not work for GET, DELETE and such. |
Swagger editor supports this functionality by converting the body into url encoded query string. Is this a possible approach to update the Swagger UI? |
Swagger Editor has been changed and will work the same as swagger-ui. |
Right now, the Java "jaxrs" server accepts and parses the body for GET requests, but the "csharp" client doesn't send them even if it generates code that allows you to specify it. |
That's pretty much the definition of undefined behavior. In theory adding a body to a https://en.wikipedia.org/wiki/Undefined_behavior So yes, it's absolutely allowed to add a body to a GET request, but no you shouldn't and rely on anyone to accept it. Swagger-ui is also totally in-spec by rejecting it. ElasticSearch relying on this is simply bad design. |
@evert, agreed. To clarify, here's what OpenAPI 3.0 says about this:
|
I don't think enforcing "good practices" is the business of an UI tool. Whether the request body is discarded or not should be decided on the server side. |
OpenAPI 3.0 has the say here. GET bodies SHALL be ignored. |
Indeed @javabrett. OAI has made their position quite clear in the newest version of their spec, therefore Swagger UI won't be supporting this. |
IMHO this should be reopened. In the RFC, request bodies of This is of course not harmless. Typical use cases are your elastic search queries, with for example:
...but also covers all range of queries which aren't simplistic and need a bunch of dynamic parameters. For these, the choice boils down to "not use swagger/openapi" or "use POST method" which is clearly semantically wrong. |
Note: apparently, it's being changed soon: |
GET request queries can be cached, so if there is sensitive information in that string then caching servers will keep a copy if this. I would like to use a GET request because I am not changing anything in my system, but I don't want sensitive information being held on these servers :/ To me it makes sense to allow it for edge cases like this. |
well apple completely forbids a body on a get request... and http docs does say it can cause issues. So issues are starting to appear |
While I do agree it should be discouraged, disallowing it is not the responsibility of swagger-ui. The spec allows it, so should you. Just stating "use POST" is about as correct as stating "use a hammer on that screw" if you're talking about adhering to a specification... |
Using POST is absolutely more appropriate than GET for a case like this. Excerpt from the yet-unreleased new HTTP spec:
This is way clearer than previous versions of the spec, but effectively states that while a body is allowed, it is equivalent to not including a body and therefore meaningless. |
@evert Like I said, if the spec is going to completely disallow this, there are going to be complications with URL length and I hope this is also solved then. For instance ElasticSearch uses a body in a GET request because of the nested complexity. As you can read I have my grievances of putting potentially sensitive data in the request url that is arbitrarily capped at a certain length by undefined actors in the network you most likely have no influence over. Until DoH is default this should not be pushed through imho, forcing this shift might disclose a lot of data that was not visible before to the ISP and other (malicious) parties on the DNS network. |
No, don't use GET for cases where PII or URL length is a concern. Very simple. This is not going to change. |
I really do think this is a case of dogma trumping common sense. The point of having a body in a GET is for complicated read-only operations. That is why Elastic Search uses it. The POST, PUT and PATCH verbs are for changing data. The fact that most applications and servers never had a need to implement body parsing for GETs is neither here nor there. The point is that fact should not go and change the design of the protocol. The protocol allows a body because you might need it to do a query. Lots of things from SQL to GraphQL could be put in the body. And lots of systems are broken today because they use POST for doing read-only stuff, which prevents them from being able to dispatch easily and appropriately on the HTTP verb, for instance for access control (no write = NO POST, PUT). |
No, the meaning of You can argue that it's helpful to have a HTTP method that is safe, idempotent and allows request bodies for complex queries, and I would agree with you. Several HTTP methods have popped up to fill this void, and the most recent effort to generalize this is the https://tools.ietf.org/html/draft-snell-search-method So I'm not disagreeing that it can be useful to have this, it's just not correct to use But lets ignore all the standards for a sec and talk about common sense & dogma. The issue with doing this in the real world is that a ton of tools (including swagger I guess) assume request bodies on GET requests are semantically meaningless. This means that a ton of clients will not accept a body, and servers and proxies discard the body. This is not just a hypothetical, I don't even think the So from a pure practical standpoint, what is the benefit of using |
I think that confuses REST with HTTP. And again, the point is to separate the idempotent verbs from the verbs that change the content on the server. Mixing those two is explicitly not intended by the design. The primary motivation of this is the ability to clearly understand what is cacheable. The 2006 working group ended with this for GET in RFC-7231:
|
What part of the snippet you shared conflicts with my interpretation? HTTP/1.0 was earlier than REST, but REST in the first place really describes HTML/HTTP-like applications. So browsers, links, actions/forms, javascript. The first browser had an address bar, changing the address results in fetching the resource at the address using This was the first intent of HTTP, and REST attempts to describe this model in a more general way.
From the REST acronym, we have representation and transfer.. just missing state ;) The author of the REST disseration co-authored the HTTP/1.1 after, so in many ways REST informs modern HTTP, not the other way around. |
@evert I am in no camp here, just curious since I found this: OAI/OpenAPI-Specification#2117 Should they revert that change then? I think your indication of a new, future SEARCH keyword might imply so. Or maybe they want to keep allowing a body in a GET until such an RFC lands? |
I can't speak for them, but my interpretation for the change for OpenAPI is: "We acknowledge that this is terrible design and nobody should do this, but even so we want to support the people that having request bodies on GET". It wouldn't have been my decision, but I understand the motivation |
I don't think you should forcibly forbid people to do stuff that is explicitly not defined. We'll probably end up using |
Any service that needs more room than a url allows to express target resources to respond with need the space in the body. Search services like elastic and databases with HTTP interfaces are in this category. POST creates resources and so is not appropriate for an operation that creates nothing but simply returns what is already there. Now that search has recently been defined (https://annevankesteren.nl/2007/10/http-methods) maybe REST could use that. But note, it defines the body as where the query is. So it isn't different than GET with a body. The answer that is for sure not correct is sending a post and expecting to create nothing but instead do an effective get of a resource. So at that point maybe swagger ui is not the tool of choice. Maybe a broader defined tool is in a better position to be long lived and usable everywhere? There is nothing terribly special about a standard here if it is not universal. A proprietary or hand rolled format with more expressiveness (like maybe Postman et al.) make excellent clients too. |
Why does a documentation tool have such a strong opinion about how the services being documented are designed? |
Just in case people are still confused why |
It's not about whether people "should" use a body with GET.
It's recognizing the reality that some people do it despite the fact that
they shouldn't.
And not going out of our way to prevent them from modeling their "wrong"
design with swagger.
It's not swaggers job to enforce convention and that's the role being
assumed here. The spec *does not forbid* what elasticsearch is doing here,
and swagger is actively forbidding what the spec itself does not. That's a
mighty high horse to think you know so much better than everyone else to
impose your interpretation of an ambiguous spec.
…On Tue, Mar 1, 2022, 11:27 AM Evert Pot ***@***.***> wrote:
Just in case people are *still* confused why GET should not have request
bodies, I wrote even more words on my blog:
https://evertpot.com/get-request-bodies/
—
Reply to this email directly, view it on GitHub
<#2136 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIQ4BON3R4IPTK26OQGMS3U5ZVSRANCNFSM4CC3WZNA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
I consulted with the actual authors of the HTTP spec and they share this point of view. I've contributed to the new phrasing in the upcoming HTTP spec to better clarify this. These changes were accepted and it will be a lot less unclear. But sure... attack me instead of my arguments. Can this thread be locked? |
Heh, the issue is closed, any additional comments that come in are not really going to impact anything. Here's some information that people seem to be missing:
So - you can complain about Swagger all you want, but it is not Swagger's fault. If you're using OpenAPI 3.0, you can't describe payloads for GET operations. Nothing will change that. If you want to describe such operations, switch to OpenAPI 3.1. |
This is why we use Graphql instead of Swagger. There are complex scenarios you SHOULD send a request body to GET or instead you will need to use POST while you're still 'GET'ting data from server. It was simply funny to see that a DOCUMENTATION TOOL enforces this kind of thing. :D |
agreed with all you said, sir. |
Dude, just use POST instead of GET to solve the problem and after change to OpenAPI 3.1 |
using Swagger UI v5.0.0 It still fail on GET with body payload , this is no supported (even with openapi 3.1 support ) ? thanks |
the answer is there -> #8682 (comment) TLTR : Swagger UI v5 do not support GET with body payload |
swagger-ui version: 2.1.2
For HTTP GET method Swagger UI doesn't send body payload.
I prepared endpoint (products/test) with simple request data (name field).
On 'Try it out' click the request to specified endpoint is done, but body payload is not included in the request.
Swagger UI test endpoint:

GET request without body payload:

Issue is strictly related with Swagger UI, because CURL command is correct:
Execution of CURL command works as expected: body payload is send with HTTP GET method and match to REST API endpoint.
Based on the HTTP protocol specification GET method doesn't disallow body payload.
The text was updated successfully, but these errors were encountered: