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

compat: /response-headers endpoint missing headers #141

Open
Tracked by #91
sonbui00 opened this issue Aug 7, 2023 · 6 comments
Open
Tracked by #91

compat: /response-headers endpoint missing headers #141

sonbui00 opened this issue Aug 7, 2023 · 6 comments
Labels
compat Tracking issues around compatibility with original httpbin implementation

Comments

@sonbui00
Copy link

sonbui00 commented Aug 7, 2023

I got a different response.

Original
 curl https://httpbin.org/response-headers\?access_token\=faketoken\&token_type\=Bearer   
{
  "Content-Length": "128", 
  "Content-Type": "application/json", 
  "access_token": "faketoken", 
  "token_type": "Bearer"
}

go-httpbin
 curl http://httpbin:9100/response-headers\?access_token\=faketoken\&token_type\=Bearer  
{
  "access_token": [
    "faketoken"
  ],
  "token_type": [
    "Bearer"
  ]
}
@mccutchen mccutchen added the compat Tracking issues around compatibility with original httpbin implementation label Aug 7, 2023
@mccutchen mccutchen changed the title API response-headers compatibility with origin /response-headers endpoint compatibility Aug 7, 2023
@mccutchen mccutchen reopened this Aug 15, 2023
@mccutchen
Copy link
Owner

I still think this is worth fixing, at least in part. Copying and lightly adapting a comment I left on the reporter's abandoned attempt to fix this issue:


If we're aiming for compatibility, it's worth exploring what the original httpbin's behavior is in weird circumstances? Like, what happens with a request like /response-headers?Content-Length=999999999? I think we need some test coverage for at least that edge case, to codify whatever behavior we decide on.

Turns out, the behavior is interesting (TLS-related noise elided below):

$ curl -v --http1.1 'https://httpbin.org/response-headers?Content-Type=text/plain&Content-Length=99999'
*   Trying 54.210.149.139:443...
* Connected to httpbin.org (54.210.149.139) port 443 (#0)
> GET /response-headers?Content-Type=text/plain&Content-Length=99999 HTTP/1.1
> Host: httpbin.org
> User-Agent: curl/8.1.2
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Mon, 07 Aug 2023 22:40:24 GMT
< Content-Type: text/plain
< Content-Length: 99999
< Connection: keep-alive
< Server: gunicorn/19.9.0
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Credentials: true
< 
{
  "Content-Length": [
    "122", 
    "99999"
  ], 
  "Content-Type": [
    "application/json", 
    "text/plain"
  ]
}
* transfer closed with 99877 bytes remaining to read
* Closing connection 0
curl: (18) transfer closed with 99877 bytes remaining to read

I can convince myself that letting the incoming request to this specific endpoint dictate the actual Content-Length of the response could be useful for testing weird behavior in HTTP client or proxy code, so I'd be in favor of matching the Python implementation there.


Note that there's a weird chicken/egg problem with the output from original httpbin above: It has the correct final Content-Length value as the first item under that key in the response JSON, but … how could it compute the accurate content length of the response without serializing the whole thing, including the final value for Content-Length?

I'm probably overthinking it or missing something comically obvious, but it turns out their implementation of this endpoint is a weird loop, presumably to solve this problem?

@mccutchen mccutchen changed the title /response-headers endpoint compatibility compat: /response-headers endpoint missing headers Mar 18, 2024
@MasonM
Copy link

MasonM commented Mar 2, 2025

FYI: It looks like @sonbui00 entered this ticket due to Argo Workflows depending on the httpbin behavior. I found an alternative that works for Argo's particular use-case, which will allow us to switch: https://github.com/argoproj/argo-workflows/pull/14239/files#r1976546694

However, this alternative isn't compatible with httpbin, because the content-type header is different:

$ curl 'https://httpbin.org/base64/YWNjZXNzX3Rva2VuPWZha2V0b2tlbg==' -v 2>&1 | grep -i 'content-type'
< content-type: text/html; charset=utf-8
$ curl 'http://localhost:8088/base64/YWNjZXNzX3Rva2VuPWZha2V0b2tlbg==' -v 2>&1 | grep -i 'content-type'
< Content-Type: text/plain; charset=utf-8

The Go oauth2 library we're using has special behavior for text/plain, and won't work with text/html: https://github.com/golang/oauth2/blob/9c82a8cf7ac23b0c34d335ffae37db2995a5ad37/internal/token.go#L276

@mccutchen
Copy link
Owner

@MasonM thanks for following up with a bit more context. I would love to find a way to support argo's use cases, especially since I've used argo's software in my own work!

The /base64 endpoint returning Content-Type: text/html for all requests is unsafe (see #67), so I'm not going to change the current default behavior there.

But what if we made it possible for the request itself to specify the content type it wants in the response, either via a query param (e.g. /base64/YWNjZXNzX3Rva2VuPWZha2V0b2tlbg==?content-type=text/html) or maybe via the Accept header? (The latter seems more semantically appropriate, but at a glance I think only the former would work for the linked use case?)

This now seems like a generally useful and obvious-in-hindsight feature for this endpoint beyond just your use case, so if it seems reasonable to you I'd be happy to add the functionality (or review/accept a pull request adding it) — with the caveat that I won't be able to get to it till this weekend.

But first please let me know if this would actually be an effective fix for you!

@mccutchen
Copy link
Owner

The /base64 endpoint returning Content-Type: text/html for all requests is unsafe (see #67), so I'm not going to change the current default behavior there.

But what if we made it possible for the request itself to specify the content type it wants in the response, either via a query param (e.g. /base64/YWNjZXNzX3Rva2VuPWZha2V0b2tlbg==?content-type=text/html)

… thinking this through just a little bit more, while this does immediately re-open the possibility of injected XSS, it does at least make the resulting content-type explicit in the request itself, so I'm convincing myself it's reasonable feature to add!

(An httpbin-like service is in a weird spot in that it allows clients to generate potentially problematic requests by design and I don't think this reflected XSS edge case actually poses any real danger to clients, but if anyone can explain why it would I'd be happy to learn more.)

@mccutchen
Copy link
Owner

Had a bit of extra time, see #198 for what I have in mind here.

@MasonM
Copy link

MasonM commented Mar 4, 2025

@mccutchen Thanks for the quick reply! To be clear, the workaround in https://github.com/argoproj/argo-workflows/pull/14239/files#r1976546694 is enough to support our use case, though it isn't ideal. We don't need compatibility with httpbin, so the fact it returns Content-Type: text/html isn't a problem for us.

However, the workaround does rely on some hacky behavior in golang.org/x/oauth2 for text/plain responses, which are non-standard, and it means we can't test the normal behavior for application/json responses. Adding support for specifying the content type in the query string is a good idea, because then we could do /base64/eyJhY2Nlc3NfdG9rZW4iOiJmYWtldG9rZW4ifQ==?content-type=application/json to have it return {"access_token":"faketoken"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat Tracking issues around compatibility with original httpbin implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants