Skip to content

Commit c8ce65a

Browse files
mtdowlingMichael Dowling
authored andcommitted
Prevent bodies for 204/205 responses
And also mention that violating the HTTP spec in general is asking for a bad time. Closes #1244
1 parent e72486e commit c8ce65a

File tree

6 files changed

+82
-8
lines changed

6 files changed

+82
-8
lines changed

docs/source/1.0/spec/core/http-traits.rst

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,13 @@ multiple HTTP message locations (e.g., a member cannot be bound to both a URI
1212
label and to a header). Only top-level members of an operation's input, output,
1313
and error structures are considered when serializing HTTP messages.
1414

15-
.. contents:: Table of contents
16-
:depth: 1
17-
:local:
18-
:backlinks: none
15+
.. important::
16+
17+
Violating `HTTP specifications`_ or relying on poorly-supported HTTP
18+
functionality when defining HTTP bindings will limit interoperability
19+
and likely lead to undefined behavior across Smithy implementations. For
20+
example, avoid defining GET/DELETE requests with payloads, defining
21+
response payloads for operations with a 204/205 status, etc.
1922

2023

2124
.. smithy-trait:: smithy.api#http
@@ -51,7 +54,9 @@ The ``http`` trait is a structure that supports the following members:
5154
- ``integer``
5255
- The HTTP status code of a successful response. Defaults to ``200`` if
5356
not provided. The provided value SHOULD be between 100 and 599, and
54-
it MUST be between 100 and 999.
57+
it MUST be between 100 and 999. Status codes that do not allow a body
58+
like 204 and 205 SHOULD bind all output members to locations other than
59+
the body of the response.
5560

5661
The following example defines an operation that uses HTTP bindings:
5762

@@ -1332,3 +1337,4 @@ marked with the ``httpPayload`` trait:
13321337
13331338
.. _percent-encoded: https://tools.ietf.org/html/rfc3986#section-2.1
13341339
.. _HTTP request line: https://tools.ietf.org/html/rfc7230.html#section-3.1.1
1340+
.. _HTTP specifications: https://datatracker.ietf.org/doc/html/rfc7230

smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBindingIndex.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,36 @@ private String determineContentType(
386386
return null;
387387
}
388388

389+
/**
390+
* Returns true if the request has a modeled body.
391+
*
392+
* @param operation Operation to check.
393+
* @return Returns true if the operation has document or payload bindings.
394+
*/
395+
public boolean hasRequestBody(ToShapeId operation) {
396+
return hasPayloadBindings(getRequestBindings(operation).values());
397+
}
398+
399+
/**
400+
* Returns true if the response has a modeled body.
401+
*
402+
* @param operation Operation to check.
403+
* @return Returns true if the operation has document or payload bindings.
404+
*/
405+
public boolean hasResponseBody(ToShapeId operation) {
406+
return hasPayloadBindings(getResponseBindings(operation).values());
407+
}
408+
409+
private boolean hasPayloadBindings(Collection<HttpBinding> bindings) {
410+
for (HttpBinding binding : bindings) {
411+
if (binding.getLocation() == HttpBinding.Location.DOCUMENT
412+
|| binding.getLocation() == HttpBinding.Location.PAYLOAD) {
413+
return true;
414+
}
415+
}
416+
return false;
417+
}
418+
389419
private List<HttpBinding> computeRequestBindings(OperationIndex opIndex, OperationShape shape) {
390420
return createStructureBindings(opIndex.expectInputShape(shape.getId()), true);
391421
}

smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpResponseCodeSemanticsValidator.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.List;
2020
import java.util.Optional;
2121
import software.amazon.smithy.model.Model;
22+
import software.amazon.smithy.model.knowledge.HttpBindingIndex;
2223
import software.amazon.smithy.model.shapes.OperationShape;
2324
import software.amazon.smithy.model.shapes.Shape;
2425
import software.amazon.smithy.model.shapes.StructureShape;
@@ -39,7 +40,7 @@ public List<ValidationEvent> validate(Model model) {
3940
List<ValidationEvent> events = new ArrayList<>();
4041

4142
for (OperationShape operation : model.getOperationShapesWithTrait(HttpTrait.class)) {
42-
validateOperationsWithHttpTrait(operation).ifPresent(events::add);
43+
validateOperationsWithHttpTrait(model, operation).ifPresent(events::add);
4344
}
4445

4546
for (StructureShape structure : model.getStructureShapesWithTrait(ErrorTrait.class)) {
@@ -49,12 +50,21 @@ public List<ValidationEvent> validate(Model model) {
4950
return events;
5051
}
5152

52-
private Optional<ValidationEvent> validateOperationsWithHttpTrait(OperationShape operation) {
53+
private Optional<ValidationEvent> validateOperationsWithHttpTrait(Model model, OperationShape operation) {
5354
HttpTrait trait = operation.expectTrait(HttpTrait.class);
5455
if (trait.getCode() < 200 || trait.getCode() >= 300) {
5556
return Optional.of(invalidOperation(operation, trait));
5657
}
5758

59+
if (trait.getCode() == 204 || trait.getCode() == 205) {
60+
if (HttpBindingIndex.of(model).hasResponseBody(operation)) {
61+
return Optional.of(danger(operation, String.format(
62+
"The HTTP %d status code does not allow a response body. To use this status code, all output "
63+
+ "members need to be bound to @httpHeader, @httpPrefixHeaders, @httpResponseCode, etc.",
64+
trait.getCode())));
65+
}
66+
}
67+
5868
return Optional.empty();
5969
}
6070

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-request-response-validator.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@
324324
"smithy.api#http": {
325325
"method": "PUT",
326326
"uri": "/f",
327-
"code": 204
327+
"code": 201
328328
}
329329
}
330330
},
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[DANGER] smithy.example#InvalidStatusCode204: The HTTP 204 status code does not allow a response body. To use this status code, all output members need to be bound to @httpHeader, @httpPrefixHeaders, @httpResponseCode, etc. | HttpResponseCodeSemantics
2+
[DANGER] smithy.example#InvalidStatusCode205: The HTTP 205 status code does not allow a response body. To use this status code, all output members need to be bound to @httpHeader, @httpPrefixHeaders, @httpResponseCode, etc. | HttpResponseCodeSemantics
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
$version: "1.0"
2+
3+
namespace smithy.example
4+
5+
@http(uri: "/invalid204", method: "GET", code: 204)
6+
@readonly
7+
operation InvalidStatusCode204 {
8+
output: InvalidStatusCode204Output,
9+
}
10+
11+
@output
12+
structure InvalidStatusCode204Output {
13+
bad: String
14+
}
15+
16+
17+
@http(uri: "/invalid205", method: "GET", code: 205)
18+
@readonly
19+
operation InvalidStatusCode205 {
20+
output: InvalidStatusCode205Output,
21+
}
22+
23+
@output
24+
structure InvalidStatusCode205Output {
25+
bad: String
26+
}

0 commit comments

Comments
 (0)