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

webapp handle each request method appropriately #11224

Merged

Conversation

nathan-muir
Copy link
Contributor

@nathan-muir nathan-muir commented Oct 27, 2020

Hi,

This PR is just to fix some rough edges where the meteor webapp will respond to unsupported request methods with content, instead of responding appropriately.

Examples of current misbehaviour

APP="http://localhost:3000"

# Requests to known static resources 
KNOWN_ASSET="$APP/path/to/static"

# returns a 200 OK, along with the content of the static resource...
curl -I -X OPTIONS "$KNOWN_ASSET"

# returns a 200 OK, along with the HTML boilerplate
curl -I -X DELETE "$KNOWN_ASSET"

# Requests to any "unhandled url" 
# Any request method just returns a 200 OK and sends the HTML boilerplate
curl -I -X OPTIONS "$APP"
curl -I -X DELETE "$APP"
curl -I -X POST "$APP"
curl -I -X PUT "$APP"
curl -I -X PATCH "$APP"
curl -I -X TRACE "$APP"

The following scheme should improve compliance:

GET

  • Respond with the requested resource; static asset, boilerplate etc.

HEAD

  • Return headers identical to GET request
  • Do not send content (It's ignored by user agents anyway)

OPTIONS

  • Respond with 200
  • Send an Allow Header listing acceptable request methods
  • Do not send content

CONNECT, DELETE, PATCH, POST, PUT, TRACE, etc.

  • Respond with 405 Method Not Allowed
  • Send an Allow Header listing acceptable request methods
  • Do not send content

--

One issue that this PR creates, if a user has added a handler to WebAppInternals.connectHandlers, to handle a POST request body, but then still wants the boilerplate to be sent as a response. We could however, add a workaround for this case; eg, supplying an API to return the boilerplate.

--

Relevant RFC's

RFC 7231 OPTIONS

A server generating a successful response to OPTIONS SHOULD send any header fields that might indicate optional features implemented by the server and applicable to the target resource (e.g., Allow)

A server MUST generate a Content-Length field with a value of "0" if no payload body is to be sent in the response.

RFC 7231 405 Method Not Allowed

The origin server MUST generate an Allow header field in a 405 response containing a list of the target resource's currently supported methods

RFC 7231 HEAD

A payload within a HEAD request message has no defined semantics; sending a payload body on a HEAD request might cause some existing implementations to reject the request.

@filipenevola filipenevola added Project:Webapp waiting-feedback It's implemented but we need feedback that it is working as expected labels Nov 5, 2020
Fix incorrect responses, like sending content to OPTIONS requests, by implementing the following scheme:

GET
- Respond with the requested resource; static asset, boilerplate etc.

HEAD
- Return headers identical to GET request
- Do not send content (Node.js will automatically skip response content)

OPTIONS
- Respond with 200
- Send an Allow Header listing acceptable request methods
- Do not send content

CONNECT, DELETE, PATCH, POST, PUT, TRACE, etc.
- Respond with 405 Method Not Allowed
- Send an Allow Header listing acceptable request methods
- Do not send content
[RFC 7231 OPTIONS](https://tools.ietf.org/html/rfc7231#section-4.3.7)

> A server generating a successful response to OPTIONS SHOULD send any header fields that might indicate optional features implemented by the server and applicable to the target resource (e.g., Allow)

> A server MUST generate a Content-Length field with a value of "0" if no payload body is to be sent in the response.

[RFC 7231 405 Method Not Allowed](https://tools.ietf.org/html/rfc7231#section-6.5.5)

> The origin server MUST generate an Allow header field in a 405 response containing a list of the target resource's currently supported methods
@StorytellerCZ StorytellerCZ added this to the Release 2.3 milestone Apr 29, 2021
@filipenevola filipenevola mentioned this pull request May 20, 2021
6 tasks
@filipenevola filipenevola removed the waiting-feedback It's implemented but we need feedback that it is working as expected label May 20, 2021
@filipenevola filipenevola changed the base branch from devel to release-2.3 May 20, 2021 11:35
@filipenevola filipenevola merged commit 94694b1 into meteor:release-2.3 May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants