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

Should HTTPResponseTests include a Content-Length assertion? #1255

Open
hlbarber opened this issue Jun 3, 2022 · 2 comments
Open

Should HTTPResponseTests include a Content-Length assertion? #1255

hlbarber opened this issue Jun 3, 2022 · 2 comments
Labels
protocol-test New protocol tests are needed server This issue involves the specification for server software.

Comments

@hlbarber
Copy link
Contributor

hlbarber commented Jun 3, 2022

In many of the HttpRequestTests there includes a

https://github.com/awslabs/smithy/blob/d654ab1d4ef8eeb1e89fec72028b9a2b456ec347/smithy-aws-protocol-tests/model/awsJson1_1/documents.smithy#L36-L38

asserting the existence of a Content-Length header.

Should such an assertion exist for HttpResponseTests too?

https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2

A user agent SHOULD send a Content-Length in a request message when
no Transfer-Encoding is sent and the request method defines a meaning
for an enclosed payload body.

Aside from the cases defined above, in the absence of
Transfer-Encoding, an origin server SHOULD send a Content-Length
header field when the payload body size is known prior to sending the
complete header section.

seems to suggest that including a Content-Length header is recommended (but not required) for both clients and servers.

@mtdowling
Copy link
Member

Yeah they could. I was being more prescriptive on the client and less so on the service because clients really shouldn’t rely on always getting a Content-Length because it’s not mandated by HTTP.

@hlbarber
Copy link
Contributor Author

hlbarber commented Jun 4, 2022

There is also the opposite approach where test coverage doesn't cover anything outside the Smithy spec? For example, only check Content-Length in the case where @requiresLength is present. This approach might be chosen to avoid the grey area of whether or not tests should cover other SHOULDs in the HTTP spec.

Perhaps this comment is relevant.

@jvschneid jvschneid added the protocol-test New protocol tests are needed label Jul 24, 2023
@kstich kstich added the server This issue involves the specification for server software. label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol-test New protocol tests are needed server This issue involves the specification for server software.
Projects
None yet
Development

No branches or pull requests

4 participants