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

api-gateway creates inconsistent redirect responses where Content-Length is 0 but still have body #1349

Closed
precoder opened this issue Nov 20, 2024 · 2 comments · Fixed by #1350
Assignees

Comments

@precoder
Copy link
Contributor

Hello,

As we develop some integration tests for our GUI, we came accross with an issue causes by inconsistent Content-Length and Body.
Here is an example body created by redirect for the login/consent endpoint:

HTTP/1.1 307 Temporary Redirect
Location: /auth/login/consent
Content-Type: text/html;charset=UTF-8
Content-Length: 0
Expires: Tue, 03 Jul 2001 06:00:00 GMT
Cache-Control: no-store, no-cache, must-revalidate, max-age=0
Cache-Control: post-check=0, pre-check=0
Pragma: no-cache
Connection: close

<html><head><title>Moved.</title></head><body><h1>Moved.</h1><p>This page has moved to <a href="/auth/login/consent">/auth/login/consent</a>.</p></body></html>

This does not cause any trouble with normal browsers however it is still an invalid combination of Content-Length and Body, therefore the Test framework do not want to accept this.

On the code I have seen this is caused by redirectToConsentPage method:
https://github.com/membrane/api-gateway/blob/master/core/src/main/java/com/predic8/membrane/core/interceptor/oauth2/processors/EmptyEndpointProcessor.java#L123

Response.redirect creates a Body as can be seen above but then "bodyEmpty()" sets the Content-Length to 0 without removing the Body.

I have seen other usages of Response.redirect where ".body("")" is called
https://github.com/membrane/api-gateway/blob/master/core/src/main/java/com/predic8/membrane/core/interceptor/oauth2/flows/CodeFlow.java#L67

bodyEmpty() method is actually a better approach but it should also clear the Body after setting the Content-Length to 0 otherwise it creates inconsistent responses.

precoder pushed a commit to precoder/service-proxy that referenced this issue Nov 20, 2024
@predic8
Copy link
Member

predic8 commented Nov 21, 2024

@precoder thanks for the issue and the description. We will fix that.

predic8 added a commit that referenced this issue Nov 23, 2024
…h. (#1350)

* Fix issue #1349 by setting body to empty string after 0 content-length.

* Added Tests

---------

Co-authored-by: Mehmet Can Cömert <mehmet.coemert@kisters.de>
Co-authored-by: Thomas Bayer <bayer@predic8.de>
@predic8
Copy link
Member

predic8 commented Nov 26, 2024

Fixed in master

@predic8 predic8 closed this as completed Nov 26, 2024
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 a pull request may close this issue.

3 participants