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

Prevent bodies for 204/205 responses #1254

Merged
merged 1 commit into from
Jun 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 11 additions & 5 deletions docs/source/1.0/spec/core/http-traits.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ multiple HTTP message locations (e.g., a member cannot be bound to both a URI
label and to a header). Only top-level members of an operation's input, output,
and error structures are considered when serializing HTTP messages.

.. contents:: Table of contents
:depth: 1
:local:
:backlinks: none
.. important::

Violating `HTTP specifications`_ or relying on poorly-supported HTTP
functionality when defining HTTP bindings will limit interoperability
and likely lead to undefined behavior across Smithy implementations. For
example, avoid defining GET/DELETE requests with payloads, defining
response payloads for operations with a 204/205 status, etc.


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

The following example defines an operation that uses HTTP bindings:

Expand Down Expand Up @@ -1332,3 +1337,4 @@ marked with the ``httpPayload`` trait:

.. _percent-encoded: https://tools.ietf.org/html/rfc3986#section-2.1
.. _HTTP request line: https://tools.ietf.org/html/rfc7230.html#section-3.1.1
.. _HTTP specifications: https://datatracker.ietf.org/doc/html/rfc7230
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,36 @@ private String determineContentType(
return null;
}

/**
* Returns true if the request has a modeled body.
*
* @param operation Operation to check.
* @return Returns true if the operation has document or payload bindings.
*/
public boolean hasRequestBody(ToShapeId operation) {
return hasPayloadBindings(getRequestBindings(operation).values());
}

/**
* Returns true if the response has a modeled body.
*
* @param operation Operation to check.
* @return Returns true if the operation has document or payload bindings.
*/
public boolean hasResponseBody(ToShapeId operation) {
return hasPayloadBindings(getResponseBindings(operation).values());
}

private boolean hasPayloadBindings(Collection<HttpBinding> bindings) {
for (HttpBinding binding : bindings) {
if (binding.getLocation() == HttpBinding.Location.DOCUMENT
|| binding.getLocation() == HttpBinding.Location.PAYLOAD) {
return true;
}
}
return false;
}

private List<HttpBinding> computeRequestBindings(OperationIndex opIndex, OperationShape shape) {
return createStructureBindings(opIndex.expectInputShape(shape.getId()), true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.List;
import java.util.Optional;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.knowledge.HttpBindingIndex;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.StructureShape;
Expand All @@ -39,7 +40,7 @@ public List<ValidationEvent> validate(Model model) {
List<ValidationEvent> events = new ArrayList<>();

for (OperationShape operation : model.getOperationShapesWithTrait(HttpTrait.class)) {
validateOperationsWithHttpTrait(operation).ifPresent(events::add);
validateOperationsWithHttpTrait(model, operation).ifPresent(events::add);
}

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

private Optional<ValidationEvent> validateOperationsWithHttpTrait(OperationShape operation) {
private Optional<ValidationEvent> validateOperationsWithHttpTrait(Model model, OperationShape operation) {
HttpTrait trait = operation.expectTrait(HttpTrait.class);
if (trait.getCode() < 200 || trait.getCode() >= 300) {
return Optional.of(invalidOperation(operation, trait));
}

if (trait.getCode() == 204 || trait.getCode() == 205) {
if (HttpBindingIndex.of(model).hasResponseBody(operation)) {
return Optional.of(danger(operation, String.format(
"The HTTP %d 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.",
trait.getCode())));
}
}

return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@
"smithy.api#http": {
"method": "PUT",
"uri": "/f",
"code": 204
"code": 201
}
}
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[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
[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 number Diff line number Diff line change
@@ -0,0 +1,26 @@
$version: "1.0"

namespace smithy.example

@http(uri: "/invalid204", method: "GET", code: 204)
@readonly
operation InvalidStatusCode204 {
output: InvalidStatusCode204Output,
}

@output
structure InvalidStatusCode204Output {
bad: String
}


@http(uri: "/invalid205", method: "GET", code: 205)
@readonly
operation InvalidStatusCode205 {
output: InvalidStatusCode205Output,
}

@output
structure InvalidStatusCode205Output {
bad: String
}