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

Ignore Range header when the method is different than GET #2710

Merged
merged 4 commits into from
Apr 3, 2023

Conversation

laurenceisla
Copy link
Member

@laurenceisla laurenceisla commented Mar 21, 2023

Closes #2705

  • Ignore Range header when method is different than GET
  • Add/modify tests
  • Refactor/clean code
  • Add changelog

@laurenceisla
Copy link
Member Author

My current approach is based on this convo which led to the conclusion that we should disallow the Range header altogether for mutation requests (mostly for POST requests).

Now, I have a doubt with what the RFC mentions:

A server MUST ignore a Range header field received with a request method that is unrecognized or for which range handling is not defined. For this specification, GET is the only method for which range handling is defined.

I understand that "ignore" means we could continue the request as if the header wasn't there? But it could be argued that we do define Range handling by returning a 400 status like we do with PUT and now, in this PR. Or am I wrong in any of those cases?

@steve-chavez
Copy link
Member

My current approach is based on #2156 (comment) which led to the conclusion that we should disallow the Range header altogether for mutation requests (mostly for POST requests).

Agree.

A server MUST ignore a Range header field received with a request method that is unrecognized or for which range handling is not defined. For this specification, GET is the only method for which range handling is defined.

Agree as well. MUST for RFCs mean it's REQUIRED, so we should follow that behavior.

Really Range is quite broken even now at the master version. Which reminds me of #2204, we should also merge that one for this release and label the breaking change.

So yes, Range should be ignored for anything besides GET. The ?limit/offset thing can behave differently.

@laurenceisla laurenceisla changed the title fix: do not allow Range header in PATCH/DELETE Ignore Range header when the method is different than GET Mar 31, 2023
@laurenceisla laurenceisla marked this pull request as ready for review March 31, 2023 20:05
CHANGELOG.md Outdated Show resolved Hide resolved
@laurenceisla laurenceisla merged commit 1aed55b into PostgREST:main Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix bug with limited PATCH or DELETE with Range header
2 participants