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

Don't copy the decission endpoint request's Content-Length #422

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

Marlinc
Copy link
Contributor

@Marlinc Marlinc commented Apr 27, 2020

We currently copy all original request headers send to the decission
endpoint back. This can include the Content-Length header which
describes the request body or response. Including the original
request Content-Length causes issues for the decission endpoint
client if the response body doesn't match the exact size.

Proposed changes

This change makes sure the Content-Length doesn't get included in
the response body and adds a test to prevent future regressions.

Checklist

  • I have read the contributing guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further comments

@Marlinc Marlinc force-pushed the decission-content-length branch from 14951a1 to 3cc22d4 Compare April 27, 2020 15:41
@Marlinc Marlinc changed the title Bug: Don't copy the decission endpoint request's Content-Length Don't copy the decission endpoint request's Content-Length Apr 27, 2020
We currently copy all original request headers send to the decission
endpoint back. This can include the Content-Length header which
describes the request body or response. Including the original
request Content-Length causes issues for the decission endpoint
client if the response body doesn't match the exact size.

This change makes sure the Content-Length doesn't get included in
the response body and adds a test to prevent future regressions.
@Marlinc Marlinc force-pushed the decission-content-length branch from 3cc22d4 to e5505ea Compare April 27, 2020 15:47
@aeneasr
Copy link
Member

aeneasr commented Apr 28, 2020

Thanks!

@aeneasr aeneasr merged commit 0e99045 into ory:master Apr 28, 2020
@Marlinc Marlinc deleted the decission-content-length branch April 28, 2020 14:10
@flusflas
Copy link
Contributor

flusflas commented Sep 29, 2020

Is this issue really solved? I have the same problem: the "Content-Length" of the request and the response are the same, even when the response has an empty body.

EDIT: I found the error. I'll create a pull request to fix it :)

flusflas added a commit to flusflas/oathkeeper that referenced this pull request Sep 29, 2020
Issue ory#422 didn't fix the problem with the requests' Content-Length
being copied in the responses because the check was case-sensitive and
unit tests didn't cover it.
flusflas added a commit to flusflas/oathkeeper that referenced this pull request Sep 29, 2020
Issue ory#422 didn't fix the problem with the requests' Content-Length
being copied in the responses because the check was case-sensitive and
unit tests didn't cover it.
aeneasr pushed a commit that referenced this pull request Sep 30, 2020
Issue #422 didn't fix the problem with the requests' Content-Length
being copied in the responses because the check was case-sensitive and
unit tests didn't cover it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants