https://kotlinlang.org logo
Join Slack
Powered by
:wave: Hi, I work on the pre-release <AWS SDK for ...
# squarelibraries
i
👋 Hi, I work on the pre-release AWS SDK for Kotlin and we use OkHttp as our default HTTP engine for sending requests to AWS services. It's a fantastic HTTP client but we've recently discovered that it does not support bodies in GET requests. It looks like there was discussion with okhttp on GH about these kinds of requests previously and the stance at the time was "It’s unlikely we’ll ever support request bodies with GET." Unfortunately, it seems that multiple AWS services have multiple GET APIs which allow (or even require) bodies. Has the stance on GET bodies changed recently? Would the OkHttp team be open to accepting a pull request that enabled GET bodies, possibly enabled by an opt-in config option?
z
Does AWS often write endpoints that don’t adhere to the HTTP spec? 😅 https://www.rfc-editor.org/rfc/rfc7231#page-24
e
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 wouldn't say it doesn't adhere to it, since it isn't forbidden. Maybe it's just ill advised
a
Does AWS often write endpoints that don’t adhere to the HTTP spec?
Out of 11k+ operations across 300+ services we have identified < 10 operations that use 
GET
with a body. There is now model validation in place to prevent introducing new APIs from using 
GET
with a body. I wouldn't call it common but some older APIs have unfortunately modeled their operations this way.
k
okhttp also doesn't allow bodies with DEL requests (bad backend devs) which I think is the right thing to do as well FWIW 🙂
e
I thought it does through a flag I was thinking of something else
i
Well "bad backend devs" may be true (or at least debatable given that the RFC does not forbid GET bodies) but the reality is those APIs exist and clients need some way of calling those services and passing bodies for some operations.
Right now the only thing we can recommend them is to use a different HTTP client than OkHttp, which we don't want to do given that OkHttp performs better than most alternatives and is supported in a wider variety of environments.
z
“Add out-of-spec support for bodies on GETs for <10 bespoke AWS endpoints” isn’t super compelling
Presumably your SDK is hiding these behind some abstraction layer, maybe you can just fall back to URLConnection or use Java’s HttpClient for those under the hood
k
Or add an endpoint abstraction layer in the backend that takes POST requests and maps them to the GET endpoints?
k
(and then deprecate the GETs lol)
i
Presumably your SDK is hiding these behind some abstraction layer, maybe you can just fall back to URLConnection or use Java’s HttpClient for those under the hood
We allow callers to configure their HTTP client as part of configuring their SDK client. Falling back to another client would ignore their custom OkHttp settings and may behave in ways they don't expect. Ideally, we'd like to use one HTTP client for all requests from a configured SDK client.
z
Then what Ken said
i
out-of-spec support for bodies on GETs
Are bodies on GETs out of spec? My reading of the HTTP RFCs says "no".
z
“Add debatably-out-of-spec support for bodies on GETs for <10 bespoke AWS endpoints” doesn’t exactly sound better heh
I think Ken’s offered a pretty reasonable solution that y’all haven’t acknowledged for some reason
a
It's one thing to enforce defaults, it's another to outright prevent making requests (however debatably out of spec we want to call them).
Well right now we are exploring both options but these aren't APIs we (the AWS SDK for Kotlin team) own. Updating the endpoints may or may not be easy/feasible/etc and requires coordination of quite a few teams. We were exploring possible solutions from both ends and wanted to inquire how open OkHttp is to allowing an opt-in path to making GET requests with bodies (we agree the default should be to not allow and that it isn't good API design but reality is what it is).
z
I don’t think that really helps tbh. It’s taking your organizational tech debt and making it OkHttp’s problem, and pushing an anti-pattern into an ubiquitous (and influential) library’s API
k
Also puzzled why those APIs aren't behind an API gateway that can do that mapping 🤔 Shouldn't have to change the APIs endpoints themselves (though that's the better fix)
y
It's been debated numerous times, but decision always fell with not supporting it. The compelling evidence to add it would be widespread support and adoption by browsers or python libraries and standard server implementations.
There is an element of backpressure on non standard servers to fix this sort of usage.
i
Support for GET+body exists in widely-used clients like curl, Python's urllib3, and .NET's standard HttpClient. While we are pursuing changes to the endpoints themselves, we're having a hard time establishing agreement given that it's not prohibited by RFCs and many existing HTTP clients already support these APIs.
y
Well I think we are all inline with the spec
sending a payload body on a GET request might cause some existing
implementations to reject the request.
i
That's server-side implementations right? A client sending a body on a get request couldn't cause a client to reject the request as far as I understand.
y
Maybe, I mean it's not clear from th3 spec, and it's not a commonly used path that demands support. So I doubt it will change in OkHttp soon. Be nice if the spec was clarified if its intended to be supported. I wonder what's the effect on caching etc.
k
Also found that the popular JS client lib Axios prevents you from sending a body with GET requests too. The http/2 spec made sending bodies explicitly invalid too
Axios = okhttp in JS land... It's really popular
i
The clearest indication of spec support I can find is in RFC 9110 § 9.3.1:
A client SHOULD NOT generate content in a GET request unless it is made directly to an origin server that has previously indicated, in or out of band, that such a request has a purpose and will be adequately supported.
In this wording, "previously indicated...that such a request has a purpose and will be adequately supported" seems to be a reason when GET bodies should be allowed. Otherwise, clients shouldn't just send a GET body to a specific endpoint and hope it can deal with it.
As far as caching, I'd expect GETs which are semantically equivalent (i.e., identical including the body) to be cacheable. But maybe that should be configurable. Or maybe even GETs with bodies should not be cached. I don't have a strong opinion on that.
As I indicated above, we're willing to do the work and submit a PR for approval. We can even gate it behind some opt-in client configuration if that's preferable. But we don't want to waste time on prepping a PR if there's no buy-in from the team maintaining OkHttp.
y
It's been discussed before and explicitly decided not to support. And OkHttp doesn't really opt for new config APIs to enable alternate compliance. So I'd advise against the PR for now.
@kenkyee thanks for the Http/2 reference https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.3
```An HTTP GET request includes request header fields and no payload
body and is therefore transmitted as a single HEADERS frame```
This would also make it tougher, a client would only know if the request can be transmitted by forcing HTTP/1.1 for the client.
i
Okay, got it. Thanks for the discussion everyone! 🙂
j
My reluctant lance to support this is motivated by correctness rather than purity. I think the specs are nice to follow (!), but we have gone for consistency with browsers and other user agents when they’re in tension with the specs. For example, we accept unescaped 
|
in URLs cause browsers do, and it’s occasionally used, cause that’s what browsers do.
Now why refuse to do GET with a body? Mostly cause there’s a bunch of special cases in OkHttp around GET that would need to be revisited.
First one is caching. We could just never cache GET with body, cause it’s a lot of work & complexity to store the request body in our cache data structure. But presumably the only reason these APIs wanted to use GET was cause GET is cacheable. Either way, I think we could disable caching and that would be fine. Probably want a test to confirm we do something reasonable on endpoints that have optional bodies. Ugh.
Next is idempotence. We skip our aggressive connection health checks on GET requests cause if the socket was quietly closed we can safely retry. We can assume these body-carrying GETs are also idempotent, but it’s also annoying cause we’ll end up streaming the request body 2x if the retry is necessary. That’s a weird edge case that is an occasional source of headaches.
Redirects are also weird for anything with a body. For some statuses we strip the body, and for others we don’t. What happens with GET? Another yuck.
Gateways and middleboxes are likely to interfere here. If we make a request through a proxy, will it pass through the body? Or do we risk HTTP request tearing? It’s a scary security concern, particularly if your GET body is itself a well-formed HTTP request.
If you own your environment you can defend against this, but for clients of cloud services there’s likely a mix of middleboxes that love and hate GET. All the GET-only endpoints I’ve heard of eventually add support for POST for this and to better support browser clients.
So I’d like to push back on request bodies on GETs as it’s just a mess of special cases that make lots of operations work. I bet if you ask the corresponding service owners they might tell you they already support POST as well.
y
The Http/2 spec is also troubling, would it only work for Http/1.1?
a
The Http/2 spec is also troubling, would it only work for Http/1.1?
There are already parts of OkHttp that are only compatible on H2 (e.g. 
RequestBody.isDuplex
) 🤷 . Our inclination here was that if OkHttp was open to a path to allowing this it would be an opt-in thing (basically saying "yes I understand this isn't universally supported, I know what I'm doing").
I bet if you ask the corresponding service owners they might tell you they already support POST as well.
If only this were true 🙂 We appreciate your position and all of the feedback. We are burning this issue from both ends, we just wanted to open up a dialogue about the implementation w.r.t the RFC. From our interpretation of the RFC if clients and servers agree to 
GET
with body then it's fine, just don't expect universal support. We don't take this to mean that clients should prevent it (for HTTP/1.1 at least) just that server/proxy support may be a problem. I think we have our answer though so as Ian said thanks for the discussion!
j
Consider caching at the application layer?
OkHttp’s cache best fits static assets like images
The problem with caching GET bodies is OkHttp would have to store those
(so that it could identify when two requests are the same)
m
Thank you, I got what I wanted working with caching at the application level.
39 Views
Open in Slack
PreviousNext
Powered by