Pleroma MediaProxy: missing content-length header when switching to Bandit
feld opened this issue · 15 comments
Hello!
I'm in the progress of making Pleroma compatible with Bandit as I think we'll get some performance gains out of it, but I've stumbled upon one interesting bug: our media proxy behaves differently with Bandit as the content-length header is absent.
The media proxy is a way for us to proxy requests to media hosted on other Fediverse servers without downloading a copy of it permanently and also shielding the user's IP address/location from being exposed. We fetch the content, clean up the headers so it's RFC compliant when proxying, and stream it out to the client. This works fine with Cowboy but with Bandit the content-length header is missing. With direct connections from browsers there is no noticeable impact, but if you put Varnish in front it to get some caching performance it starts throwing 500 errors and I believe the reason is the missing content-length header which Varnish does not tolerate.
Pleroma MediaProxy proxy code: https://git.pleroma.social/pleroma/pleroma/-/blob/develop/lib/pleroma/reverse_proxy.ex?ref_type=heads
example responses
Cowboy adapter:
HTTP/2 200
accept-ranges: bytes
access-control-allow-credentials: true
access-control-allow-origin: *
access-control-expose-headers: Link,X-RateLimit-Reset,X-RateLimit-Limit,X-RateLimit-Remaining,X-Request-Id,Idempotency-Key
cache-control: public, max-age=1209600
content-length: 37766
content-security-policy: sandbox;
content-type: image/png
date: Wed, 14 Feb 2024 14:39:59 GMT
etag: "fb8a75442fb062db673ae72c69bfd937"
last-modified: Wed, 14 Feb 2024 05:55:49 GMT
permissions-policy: interest-cohort=()
referrer-policy: same-origin
server: Cowboy
x-content-type-options: nosniff
x-download-options: noopen
x-frame-options: DENY
x-permitted-cross-domain-policies: none
x-request-id: F7PBpWHLZRfu_4UAABPE
x-xss-protection: 1; mode=block
Bandit adapter:
HTTP/2 200
cache-control: public, max-age=1209600
access-control-allow-origin: *
access-control-expose-headers: Link,X-RateLimit-Reset,X-RateLimit-Limit,X-RateLimit-Remaining,X-Request-Id,Idempotency-Key
access-control-allow-credentials: true
x-xss-protection: 1; mode=block
x-permitted-cross-domain-policies: none
x-frame-options: DENY
x-content-type-options: nosniff
referrer-policy: same-origin
x-download-options: noopen
content-security-policy: sandbox;
permissions-policy: interest-cohort=()
x-request-id: F7PBlQtIwsRtKFQAADxB
date: Wed, 14 Feb 2024 14:38:49 GMT
content-type: image/png
etag: "fb8a75442fb062db673ae72c69bfd937"
last-modified: Wed, 14 Feb 2024 05:55:49 GMT
accept-ranges: bytes
Do you know if this is happening on HTTP/1 or HTTP/2? Which version of bandit it is?
My guess that it's because you're chunking your responses over HTTP/2. On HTTP/1, calling send_chunked sets the transfer-encoding header & causes the response body to be chunk encoded. HTTP/2 specifically deprecates that header and the notion of transfer encoding, so we just ship it out on HTTP/2 without a content-length header (which is in-spec per RFC9113).
Bandit version is 1.2.1
Elixir 1.14.5. OTP25
This configuration is Phoenix/Bandit listening on 0.0.0.0:4000, so it's HTTP1 with Caddy in front doing HTTPS / HTTP2
Are those header lists above the ones returned downstream to your clients from Caddy, or is that exactly what Cowboy/Bandit is sending to Caddy?
If you're connecting to Cowboy / Bandit over HTTP/1 to the app you link above, they both ought to be sending transfer-encoding along with chunked responses.
Are those header lists above the ones returned downstream to your clients from Caddy, or is that exactly what Cowboy/Bandit is sending to Caddy?
Downstream from Caddy. Let me hit Phoenix directly and compare what it's returning. I should have done that, but I'm still on my first cup of coffee :)
Oh I think I see what's happening here. tl;dr I think you've been relying on out of spec behaviour by Cowboy this whole time.
Your app sets the content-length header on the response, by copying it through from the proxied upstream. You then send the response down to your downstream by using chunked encoding (which, for HTTP/1 at least, will set the transfer-encoding header). This combination of both content-length and transfer-encoding is specifically disallowed by RFC9112§6.2. Bandit respects this by removing any content-length header that the user sets.
Bandit:
> Host: localhost:4000
> User-Agent: curl/8.5.0
> Accept: */*
>
< HTTP/1.1 200 OK
< transfer-encoding: chunked
< cache-control: public, max-age=1209600
< access-control-allow-origin: *
< access-control-expose-headers: Link,X-RateLimit-Reset,X-RateLimit-Limit,X-RateLimit-Remaining,X-Request-Id,Idempotency-Key
< access-control-allow-credentials: true
< x-xss-protection: 1; mode=block
< x-permitted-cross-domain-policies: none
< x-frame-options: DENY
< x-content-type-options: nosniff
< referrer-policy: same-origin
< x-download-options: noopen
< content-security-policy: sandbox;
< permissions-policy: interest-cohort=()
< x-request-id: F7PFt1C5-uKvy8wAAYcC
< date: Wed, 14 Feb 2024 15:54:35 GMT
< content-type: image/jpeg
< content-length: 5263093
< accept-ranges: bytes
< etag: "3242412"
Cowboy:
> Host: localhost:4000
> User-Agent: curl/8.5.0
> Accept: */*
>
< HTTP/1.1 200 OK
< accept-ranges: bytes
< access-control-allow-credentials: true
< access-control-allow-origin: *
< access-control-expose-headers: Link,X-RateLimit-Reset,X-RateLimit-Limit,X-RateLimit-Remaining,X-Request-Id,Idempotency-Key
< cache-control: public, max-age=1209600
< content-length: 5263093
< content-security-policy: sandbox;
< content-type: image/jpeg
< date: Wed, 14 Feb 2024 15:55:59 GMT
< etag: "3242412"
< permissions-policy: interest-cohort=()
< referrer-policy: same-origin
< server: Cowboy
< x-content-type-options: nosniff
< x-download-options: noopen
< x-frame-options: DENY
< x-permitted-cross-domain-policies: none
< x-request-id: F7PFyvYbOWh7jB0AAARG
< x-xss-protection: 1; mode=block
Your app sets the content-length header on the response, by copying it through from the proxied upstream.
ahh, I disabled copying that header in and now Varnish is happy with the responses.
Thank you for your time
I'm actually going to reopen this; I noticed a couple of discrepancies in those header lists I'd like to look into
Thanks for the report!
Huh. TIL (from the Plug docs):
When using HTTP/1.1, the Cowboy adapter will stream the response instead of emitting chunks if the content-length header has been set before calling send_chunked/2.
So that behaviour is actually expected for Cowboy: even though you're calling send_chunked/2 and chunk/2, it's just sending those out as non transfer-encoded packets within a content-length delimited response. That's actually pretty smart.
In contrast, it's actually Bandit that does the wrong thing here (I don't actually remove the content-length header on chunk sends, though i'm 95% sure I once did. I think it may have regressed in the 1.0-pre series).
I'm going to copy Cowboy's behaviour as part of my HTTP/1 refactor (#297). Thanks for pointing this out!
In contrast, it's actually Bandit that does the wrong thing here (I don't actually remove the content-length header on chunk sends
what does the HTTP spec require?
The combination of both content-length and transfer-encoding is specifically disallowed by RFC9112§6.2
I've seen increased reports of issues with broken images during reverse proxying and I think I've tracked it down to errors (timeouts most likely) and progressive JPEGs when the response is being streamed by chunking. But we didn't seem to have this problem with Cowboy. A browser and HTTP caching proxy seem to happily accept broken progressive JPEGs and I'm not certain why as the transfer should not have been completed with the required closing 0\r\n\r\n ?? (Maybe that's a different bug I need to chase.)
SoI was doing more reserach on this as I wanted to get away from chunking the responses. Then I discovered that Cowboy has a special capability for streaming the response body without it being chunked when the content-length header is present! This seems to (nearly) explain everything!
I'm wondering if this is something that would be considered for Bandit
Yep, I have plans for exactly that ability. I'm expecting to have it out 'soon' (it's near the top of my list, but it's a slow moving list as of late.
I see now you mentioned that above and somehow I missed it back then. Sorry for the noise