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
fix: setRequestInterception and setExtraHTTPHeaders not working together (#1729) #1734
fix: setRequestInterception and setExtraHTTPHeaders not working together (#1729) #1734
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @yujiosaka for taking time to work on this.
lib/NetworkManager.js
Outdated
@@ -569,12 +569,16 @@ function generateRequestHash(request) { | |||
}; | |||
|
|||
if (!normalizedURL.startsWith('data:')) { | |||
const headers = Object.keys(request.headers); | |||
headers.sort(); | |||
for (const header of headers) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we please minimize the change? I believe you can add a header.toLowerCase()
as the first line in the loop:
for (let header of headers) {
header = header.toLowerCase();
if (key === 'accept' || key === 'referer' ...)
}
@@ -1321,6 +1321,16 @@ describe('Page', function() { | |||
const response = await page.goto(server.EMPTY_PAGE); | |||
expect(response.ok()).toBe(true); | |||
}); | |||
it('should works with customizing referer headers', async({page, server}) => { | |||
await page.setExtraHTTPHeaders({ 'referer': server.EMPTY_PAGE }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a good test, thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yujiosaka I've slightly changed your code; I hope you don't mind. Thank you for fixing this!
@aslushnikov thanks! |
…her (puppeteer#1734) This patch starts lowering header keys while generating request hashes. Fixes puppeteer#1729.
This is just a fix to the problem that
page.goto
resolves tonull
.Setting referer does not work properly, but that should be discussed here.
Fixes: #1729