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

🐛 Pass auth headers when requesting assets directly #977

Merged
merged 1 commit into from
Jun 30, 2022

Conversation

Robdel12
Copy link
Contributor

What is this?

In #921 we realized font responses were being corrupted through the browsers request interception API. To get around that, we now request and save fonts straight from node as a binary (no encoding). In that PR, I forgot to pass the auth config to the request that's being made, which will prevent sites behind auth from capturing fonts properly.

This PR adds the request headers the browser has to the node request we made to capture the fonts directly. Also adds a debug log when we are requesting these fonts directly.

Closes #969

@Robdel12 Robdel12 added the 🐛 bug Something isn't working label Jun 30, 2022
@Robdel12 Robdel12 requested a review from wwilsman June 30, 2022 19:00
@Robdel12 Robdel12 force-pushed the rd/pass-auth-to-node-request branch from e0e8eee to 70b2128 Compare June 30, 2022 19:05
Comment on lines 878 to 889
<html>
<head>
<style>
@font-face { font-family: "test"; src: url("font-auth/font.woff") format("woff"); }
body { font-family: "test", "sans-serif"; }
</style>
</head>
<body>
<p>Hello Percy!<p>
${' '.repeat(1000)}
</body>
</html>
Copy link
Contributor

Choose a reason for hiding this comment

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

Very small nitpick, these all look like they could be indented one more space.

Right now it's indented 7 spaces, when you probably intended it to be an even 8.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing gets past this guy

@Robdel12 Robdel12 force-pushed the rd/pass-auth-to-node-request branch from 70b2128 to 955be45 Compare June 30, 2022 19:08
Copy link
Contributor

@wwilsman wwilsman left a comment

Choose a reason for hiding this comment

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

🎮

@Robdel12 Robdel12 merged commit 8fc2cac into master Jun 30, 2022
@Robdel12 Robdel12 deleted the rd/pass-auth-to-node-request branch June 30, 2022 19:19
@andersrehn
Copy link

@Robdel12 thank you for finding and solving this bug, my project was suffering from this exact problem. Tested @percy/cli 1.6.1 and my woff2 fonts behind basic auth works like a charm again! Great work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Pass auth config to node requests for fonts
3 participants