Skip to content

fix(aspnetcore): MapCommand ignores route values and requires a request body on DELETE #848

Description

@samtrion

User Story

As a developer exposing commands through MapCommand on routes such as /orders/{id}, I want the route value to be bound to the command and body-less DELETE requests to be accepted, so that the command always targets the resource named in the URI and standard HTTP clients work without sending a body.


Problem

Both MapCommand overloads bind the command only from the request body:

  • src/NetEvolve.Pulse.AspNetCore/EndpointRouteBuilderExtensions.cs:77 and :133: async ([FromBody] TCommand command, IMediator mediator, CancellationToken cancellationToken) =>

Route and query values are never read. MapQuery (:180) and MapStreamQuery (:232) bind with [AsParameters], so the two sides behave differently.

The documentation implies that {id} fills the command:

  • XML examples: EndpointRouteBuilderExtensions.cs:55 (MapCommand<UpdateOrderCommand, OrderResult>("/orders/{id}", CommandHttpMethod.Put)) and :108 / :111 (MapCommand<DeleteOrderCommand>("/orders/{id}", CommandHttpMethod.Delete)).
  • src/NetEvolve.Pulse.AspNetCore/README.md:59-60, :96, :99, :117, :225, :248-249 map /orders/{id} or /{id}. README.md:104 declares record UpdateOrderCommand(Guid Id, ...) and README.md:122 declares record DeleteOrderCommand(Guid Id) : ICommand;.

Failure scenarios:

  1. DELETE /orders/3fa85f64-... with no body is rejected before the handler runs. Without a Content-Type header, the client usually gets 415 Unsupported Media Type. With Content-Type: application/json and an empty body, it gets 400 Bad Request, because the non-nullable [FromBody] parameter is required.
  2. PUT /orders/A with body {"id":"B", ...} dispatches the command with Id = B. The route segment A is matched and then discarded, so the command can act on a different resource than the URI targets. If authorization is based on the route, this is an IDOR-style risk.

The unit tests (tests/NetEvolve.Pulse.Tests.Unit/AspNetCore/EndpointRouteBuilderExtensionsTests.cs:104, :160) only check endpoint metadata for DELETE. They do not cover binding.


Specification

RFC 9110 §9.3.5 DELETE:

Although request message framing is independent of the method used, content received in a DELETE request has no generally defined semantics, cannot alter the meaning or target of the request, and might lead some implementations to reject the request and close the connection because of its potential as a request smuggling attack. A client SHOULD NOT generate content in a DELETE request unless it is made directly to an origin server that has previously indicated, in or out of band, that such a request has a purpose and will be adequately supported.

Pulse acts as the origin server and documents body binding, so a DELETE with a body does not strictly violate the RFC. The defect is a contract mismatch: the documented {id} examples and the RFC-recommended body-less DELETE do not work.

Microsoft Learn, Parameter binding in Minimal API apps:

Parameters declared in route handlers are treated as required ... Failure to provide all required parameters results in an error.

The HTTP methods GET, HEAD, OPTIONS, and DELETE don't implicitly bind from body. To bind from body ... bind explicitly with [FromBody].


Requirements

  • DELETE commands can be sent without a request body. Bind them with [AsParameters], as MapQuery does, so that route and query values fill the command.
  • Choose the binding per HTTP method. [AsParameters] alone would drop body binding for POST, PUT and PATCH. For those methods, either keep body binding and document that route values are ignored, or merge route values over the body (the route value wins).
  • If route values stay ignored for body-bound methods, the XML documentation and README examples must not suggest that {id} fills Id.
  • Update the XML documentation (EndpointRouteBuilderExtensions.cs) and src/NetEvolve.Pulse.AspNetCore/README.md to describe the binding source per method.
  • Mark any change to the binding behavior as a breaking change if existing clients that send a DELETE body are affected.

Acceptance Criteria

  • A failing integration test first: DELETE /orders/{id} with no body and no Content-Type reaches the handler with Id taken from the route (currently 415/400).
  • An integration test covers PUT /orders/{id} with a mismatched body Id and asserts the documented behavior (the route value wins, or the mismatch is rejected).
  • POST, PUT and PATCH commands still bind from the body.
  • The XML examples and README examples match the actual binding behavior.

No activity

Activity on this issue will appear here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type:bugIndicates an issue or flaw that needs to be fixed.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions