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

BUG: lambdaurl.Wrap does not add Content-Type or Content-Length headers like http.ResponseWriter does #508

Closed
xremming opened this issue May 6, 2023 · 3 comments
Assignees
Labels
type/lambdaurl Issue or feature request related to the lambdaurl package type/ux

Comments

@xremming
Copy link

xremming commented May 6, 2023

The http.ResponseWriter implementation says the following thing about Content-Type header (and it actually has a note about Content-Length too).

If WriteHeader has not yet been called, Write calls
WriteHeader(http.StatusOK) before writing the data. If the Header
does not contain a Content-Type line, Write adds a Content-Type set
to the result of passing the initial 512 bytes of written data to
DetectContentType. Additionally, if the total size of all written
data is under a few KB and there are no Flush calls, the
Content-Length header is added automatically.

As of right now the lambdaurl.Wrap implementation completely misses this and causes all content without an explicit Content-Type header to get it with application/json. Which is different and incorrect when compared to how the http.Handler normally works.

A simple repro for this is to use http.FileServer with e.g. HTML files. The HTML files will not be loaded by the browser properly as the response will have Content-Type: application/json header in it but when running the same handler normally it will work fine.

@xremming xremming changed the title BUG: lambdaurl's wrap implementation looses Content-Type header BUG: lambdaurl.Wrap implementation does not add Content-Type or Content-Length headers like http.ResponseWriter does May 6, 2023
@xremming xremming changed the title BUG: lambdaurl.Wrap implementation does not add Content-Type or Content-Length headers like http.ResponseWriter does BUG: lambdaurl.Wrap implementation does not add Content-Type or Content-Length headers like http.ResponseWriter does May 6, 2023
@xremming xremming changed the title BUG: lambdaurl.Wrap implementation does not add Content-Type or Content-Length headers like http.ResponseWriter does BUG: lambdaurl.Wrap does not add Content-Type or Content-Length headers like http.ResponseWriter does May 6, 2023
@bmoffatt
Copy link
Collaborator

bmoffatt commented May 7, 2023

Ooof, I thought that the Function URL backend would at least be defaulting to application/octet-stream, getting application/json as a default is confusing.

For this package, I don't think I want to attempt being 100% compatible with the http.ListenAndServe behavior, specifically the Flusher interface. The backend for Function URLs has it's own size and time based logic for flushing out to the client, so mimicking Go's http server defaults ontop of that seems like a footgun. There's other stuff like Trailer that the stdlib documents, that isn't feasible to make work for Lambda Function URLs.

Calling http.DetectContentType on the first Write ought to resolve most of the problem, at minimum getting rid of the default application/json, though again, since there'll be no internal buffering, I'm sure it'll be possible to contruct a case where the detected content types differ.

@bmoffatt bmoffatt added type/ux type/lambdaurl Issue or feature request related to the lambdaurl package labels May 7, 2023
@bmoffatt bmoffatt self-assigned this May 7, 2023
@xremming
Copy link
Author

Any timeline on when this might be fixed? This is blocking my project at the moment and I would be happy to know whether I should just implement a fix on my end or to wait for the upstream fix.

@bmoffatt
Copy link
Collaborator

Released as a part of https://github.com/aws/aws-lambda-go/releases/tag/v1.42.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/lambdaurl Issue or feature request related to the lambdaurl package type/ux
Projects
None yet
Development

No branches or pull requests

2 participants