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

Custom headers not re-used when following redirect #88

Closed
holtkamp opened this issue Nov 16, 2017 · 4 comments · Fixed by #91
Closed

Custom headers not re-used when following redirect #88

holtkamp opened this issue Nov 16, 2017 · 4 comments · Fixed by #91

Comments

@holtkamp
Copy link
Contributor

By default the Browser object follows HTTP redirect codes.

However, it seems that when doing so, any custom headers that were used on the initial request are not re-used when following the redirect. Not sure whether this should be default HTTP behaviour (but I think so?)

I suppose re-using the headers should be done here https://github.com/clue/php-buzz-react/blob/ba4eb355e9ffe4bb2a9071ccc69698d48da548dc/src/Io/Transaction.php#L136

Not sure, but maybe something like:

$request = $this->messageFactory->request($method, $location, $request->getHeaders());

The reason how I found out: we use custom headers to simulate the "Google Bot" by using a UserAgent header like Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)

After a redirect, the default UserAgent seems to be used: React/alpha defined at https://github.com/reactphp/http-client/blob/d779a3b3c91ee19742839df7395b366da4471d12/src/RequestData.php#L29

@clue
Copy link
Owner

clue commented Nov 17, 2017

Good catch and thanks for reporting! I agree that this should probably be the default behavior. Do you feel like filing a PR for this? :shipit: 👍

@holtkamp
Copy link
Contributor Author

Do you feel like filing a PR for this?

Sure!

Do you agree with the suggested approach?

$request = $this->messageFactory->request($method, $location, $request->getHeaders());

Or am I missing some details...?

@clue
Copy link
Owner

clue commented Nov 20, 2017

Sounds like a sane approach, but I'd like to see some tests for this before laying down on this 👍

@holtkamp
Copy link
Contributor Author

ok, can give it a try, but might take some time. Spare time seems scarce lately 😓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants