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 method doesn't support body payload #2136

Closed
karol-lotkowski opened this issue May 4, 2016 · 42 comments
Closed

GET method doesn't support body payload #2136

karol-lotkowski opened this issue May 4, 2016 · 42 comments

Comments

@karol-lotkowski
Copy link

karol-lotkowski commented May 4, 2016

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:
image

GET request without body payload:
image

Issue is strictly related with Swagger UI, because CURL command is correct:

curl -X GET --header "Accept: application/json" --header "Content-Type: application/json" --header "Authorization: Bearer ..." -d "{
  \"name\": \"test_name\"
}" "http://localhost:8080/application/products/test"

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.

@fehguy
Copy link
Contributor

fehguy commented May 4, 2016

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.

@lordent
Copy link

lordent commented Dec 15, 2016

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.
For example, imagine an request report with lots of fields for aggregates, filters and sorts. Yes, even given the fact that all IDs have UUID format.

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.

@ehartford
Copy link

+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)

@fehguy
Copy link
Contributor

fehguy commented Jan 5, 2017

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.

@ehartford
Copy link

Perhaps you are looking at an older version of the spec?

In the current spec:
RFC 7230 3.3

The presence of a message body in a request is signaled by a
Content-Length or Transfer-Encoding header field.  Request message
framing is independent of method semantics, even if the method does
not define any use for a message body.

The sender's allowed to send a body in a GET.

RFC 7231 4.3.1

 A payload within a GET request message has no defined semantics;
 sending a payload body on a GET request might cause some existing
 implementations to reject the request.

I don't see any mandate to disregard the body of a GET.

@scarroll32
Copy link

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.

@jconti
Copy link

jconti commented Mar 16, 2017

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?

@webron
Copy link
Contributor

webron commented Mar 16, 2017

@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.

@aseychell
Copy link

aseychell commented Mar 22, 2017

Swagger editor supports this functionality by converting the body into url encoded query string. Is this a possible approach to update the Swagger UI?

@webron
Copy link
Contributor

webron commented Mar 22, 2017

Swagger Editor has been changed and will work the same as swagger-ui.

@mhvelplund
Copy link

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.

@evert
Copy link

evert commented Sep 23, 2018

A payload within a GET request message has no defined semantics;
sending a payload body on a GET request might cause some existing
implementations to reject the request.

I don't see any mandate to disregard the body of a GET.

That's pretty much the definition of undefined behavior. In theory adding a body to a GET request could turn it into a DELETE request and it would be in-spec.

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.

@shockey
Copy link
Contributor

shockey commented Sep 25, 2018

@evert, agreed.

To clarify, here's what OpenAPI 3.0 says about this:

The requestBody is only supported in HTTP methods where the HTTP 1.1 specification RFC7231 has explicitly defined semantics for request bodies. In other cases where the HTTP spec is vague, requestBody SHALL be ignored by consumers.

@andsav
Copy link

andsav commented Feb 20, 2019

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.
In any case, allowing the user to input a request body, then discarding it without any warning is a bad idea. At least show a warning as other people suggested.

@javabrett
Copy link
Contributor

OpenAPI 3.0 has the say here. GET bodies SHALL be ignored.

@shockey
Copy link
Contributor

shockey commented Apr 5, 2019

Indeed @javabrett.

OAI has made their position quite clear in the newest version of their spec, therefore Swagger UI won't be supporting this.

@dagnelies
Copy link

dagnelies commented Jun 7, 2019

IMHO this should be reopened.

In the RFC, request bodies of GET (& co) is "undefined". In swagger/openapi, it's "forbidden".
IMHO this is overstepping the responsibility of a spec tool.

This is of course not harmless. Typical use cases are your elastic search queries, with for example:

curl -XGET 'localhost:9200/mobile/_search?pretty' -H 'Content-Type: application/json' -d'
{
  "query":{
    "bool":{ 
      "must":[
         {"terms":{"brand":["micromax","samsung"]}}
      ] , 
      "should":[
         { "range": { "price": { "gte": 6000, "lte": 10000 } } },
         { "range": { "price": { "gte": 16000, "lte": 30000 } } }
      ]
    }
  }
}

...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.

@dagnelies
Copy link

Note: apparently, it's being changed soon:

OAI/OpenAPI-Specification#1937

@olicooper
Copy link

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.

@sejolopab
Copy link

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

@svanschooten
Copy link

While I do agree it should be discouraged, disallowing it is not the responsibility of swagger-ui. The spec allows it, so should you.
The main reason is the max length of an URI is undefined and relies on the implementation of the server and all the hops in between.
Complex search requests can easily be longer than 2048 characters, which is the "customary" maximum length, as has been shown multiple times already. Using the body here makes perfect sense, though you have to be very aware that you specification can break an application.

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...

@evert
Copy link

evert commented Oct 23, 2020

Using POST is absolutely more appropriate than GET for a case like this.

Excerpt from the yet-unreleased new HTTP spec:

A client SHOULD NOT generate a body in a GET request. A payload received in a GET request has no defined semantics, cannot alter the meaning or target of the request, and might lead some implementations to reject the request and close the connection because of its potential as a request smuggling attack (Section 11.2 of [Messaging]).

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.

@svanschooten
Copy link

@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.
I do applaud the attempt to create a more defined interface, when the literal whole world run on your spec, it should be crystal clear 🙈

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.

@evert
Copy link

evert commented Nov 10, 2020

No, don't use GET for cases where PII or URL length is a concern. Very simple. This is not going to change.

@jconti
Copy link

jconti commented Nov 10, 2020

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).

@evert
Copy link

evert commented Nov 10, 2020

No, the meaning of GET is to fetch the representation of a resource at the given uri. You're extending the meaning of GET to mean a general read-only operation, but this has never been the case. The benefit of using HTTP methods correctly is so anyone implementing HTTP can make certain assumptions about how things will behave, how they can be cached, etc. By abusing GET you throw away this benefit.

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 SEARCH method, which I expect to become a RFC:

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 GET for this and a really bad idea given that the vast majority of the HTTP landscape follows the standard as intended. ElasticSearch extends HTTP in an incompatible way.

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 fetch() api in a browser allows this.

So from a pure practical standpoint, what is the benefit of using GET with request bodies? What is the gain? I'd argue that the idea that 'its a read operation, and therefore should be GET' is a more emotion driven design decision than anything, because the practical benefits aren't there either. If you do anything with consumers you don't control, you're bound to break stuff, and clearly don't care about the idempotence/caching benefits as they are not supported with bodies. It's aesthetically pleasing?

@jconti
Copy link

jconti commented Nov 10, 2020

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:

Method Definitions
Fielding & Reschke Standards Track
4.3.1. GET
The GET method requests transfer of a current selected representation
for the target resource. GET is the primary mechanism of information
retrieval and the focus of almost all performance optimizations.
Hence, when people speak of retrieving some identifiable information
via HTTP, they are generally referring to making a GET request.

It is tempting to think of resource identifiers as remote file system
pathnames and of representations as being a copy of the contents of
such files. In fact, that is how many resources are implemented (see
Section 9.1 for related security considerations). However, there are
no such limitations in practice. The HTTP interface for a resource
is just as likely to be implemented as a tree of content objects, a
programmatic view on various database records, or a gateway to other
information systems. Even when the URI mapping mechanism is tied to
a file system, an origin server might be configured to execute the
files with the request as input and send the output as the
representation rather than transfer the files directly. Regardless,
only the origin server needs to know how each of its resource
identifiers corresponds to an implementation and how each
implementation manages to select and send a current representation of
the target resource in a response to GET.

A client can alter the semantics of GET to be a "range request",
requesting transfer of only some part(s) of the selected
representation, by sending a Range header field in the request
([RFC7233]).

A payload within a GET request message has no defined semantics;
sending a payload body on a GET request might cause some existing
implementations to reject the request.

The response to a GET request is cacheable; a cache MAY use it to
satisfy subsequent GET and HEAD requests unless otherwise indicated
by the Cache-Control header field (Section 5.2 of [RFC7234]).

@evert
Copy link

evert commented Nov 11, 2020

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 GET.

This was the first intent of HTTP, and REST attempts to describe this model in a more general way.

The GET method requests transfer of a current selected representation
for the target resource. (from your snippet)

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.

@thernstig
Copy link

thernstig commented Jun 7, 2021

@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?

@evert
Copy link

evert commented Jun 7, 2021

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

@hanseartic
Copy link

I don't think you should forcibly forbid people to do stuff that is explicitly not defined. We'll probably end up using post as keyword and then just in the description write that GET should be used...

@jconti
Copy link

jconti commented Jul 18, 2021

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.

@jfaixolo
Copy link

jfaixolo commented Mar 1, 2022

Why does a documentation tool have such a strong opinion about how the services being documented are designed?

@evert
Copy link

evert commented Mar 1, 2022

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/

@ehartford
Copy link

ehartford commented Mar 1, 2022 via email

@evert
Copy link

evert commented Mar 1, 2022

That's a mighty high horse to think you know so much better than everyone else to
impose your interpretation of an ambiguous spec.

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?

@webron
Copy link
Contributor

webron commented Mar 1, 2022

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:

  1. Swagger is not the spec. It's a set of tools around the OpenAPI Specification. Swagger simply follows what OpenAPI says is or not allowed.
  2. In OpenAPI 3.0, we decided to follow the HTTP spec more closely and remove support for payloads for GET, DELETE and so on.
  3. In OpenAPI 3.1, we decided to go back on this change for the main reason that indeed many APIs are designed this way (this is not to say that we believe it's the right approach).

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.

@oereneli
Copy link

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

@esaurov
Copy link

esaurov commented Sep 2, 2022

agreed with all you said, sir.

@EufranioDiogo
Copy link

Dude, just use POST instead of GET to solve the problem and after change to OpenAPI 3.1
😄

@raphaelauv
Copy link

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

@raphaelauv
Copy link

the answer is there -> #8682 (comment)

TLTR : Swagger UI v5 do not support GET with body payload

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests