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.

Sign up for GitHub

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

Jump to bottom

Modified storing of redirect response chain #1338

Closed
wants to merge 1 commit into from
Closed

Modified storing of redirect response chain #1338

wants to merge 1 commit into from

Conversation

secinto
Copy link
Contributor

@secinto secinto commented Aug 31, 2023

Modified storing of redirect response chain to output all redirects and responses.

Previously responses (especially the final get response body) was not stored always (or even never).
Modified it that if follow redirect is specified not only the request/responses of the redirects but also the final response body is stored.

ehsandeep
ehsandeep reviewed Sep 7, 2023
View reviewed changes
Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@secinto can you share an example to reproduce this issue?

@secinto
Copy link
Contributor Author

secinto commented Sep 20, 2023

For instance if I use https://khg-graz.at:443 only the chain is stored and not the actual response.
What actually happens is that first the request and response are stored, which are thereafter overwritten by the data contained in resp.GetChain(), which doesn't contain the body of the response but only the header.

Also storing the full URL at the end (as it was introduced by remove fullurl from req-res #1337) is not adjusted currently.

Output written before it is overwritten by the current resp.GetChain() implementation.

GET / HTTP/1.1
Host: khg-graz.at
User-Agent: Mozilla/5.0 (Linux; Android 10; SM-M315F) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.50 Mobile Safari/537.36
Accept-Charset: utf-8
Accept-Encoding: gzip

HTTP/1.1 200 OK
Connection: close
Cache-Control: no-store, no-cache, must-revalidate
Content-Security-Policy: frame-ancestors 'self';
Content-Type: text/html; charset=UTF-8
...

<!DOCTYPE html>
...
<html lang="de">
<head>
<!-- PreScripts -->
...

Output after the Chain is used.

GET / HTTP/1.1
Host: khg-graz.at
Accept-Charset: utf-8
User-Agent: Mozilla/5.0 (Linux; Android 10; SM-M315F) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.50 Mobile Safari/537.36

HTTP/1.1 302 Found
Connection: close
Date: Wed, 20 Sep 2023 09:34:48 GMT
Location: https://www.khg-graz.at/
Server: Caddy
Content-Length: 0

GET / HTTP/0.0
Host: www.khg-graz.at
Accept-Charset: utf-8
Referer: https://khg-graz.at:443
User-Agent: Mozilla/5.0 (Linux; Android 10; SM-M315F) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.50 Mobile Safari/537.36

HTTP/1.1 200 OK
Connection: close
Cache-Control: no-store, no-cache, must-revalidate
Content-Security-Policy: frame-ancestors 'self';
Content-Type: text/html; charset=UTF-8
...

Thus, the actual content is missing.
My proposal would modify it that everything in the chain is present, as well as the body of the response.

Additionally adding the URL at the end can / should be added if it stays this way. Not sure if it is the best solution anyway.

@secinto secinto mentioned this pull request Sep 20, 2023
Merged
@ehsandeep
Copy link
Member

suppressed by #1373

@ehsandeep ehsandeep closed this Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehsandeep ehsandeep ehsandeep left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@secinto @ehsandeep @skraxberger