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

Update authorization header encoding #253

Merged

Conversation

julienblin-kothar
Copy link
Contributor

When running in Pyodide inside a browser, it appears that the request library puts the literal string value of headers in browser fetch requests.

By leaving the value as a byte string, it ends up being sent as Authorization: b'Basic ...'.

There are different ways to approach this isssue, but ultimately headers are strings and so this fixes the issue by decoding the value before setting the header.

@julienblin-kothar
Copy link
Contributor Author

I respectfully ask if you could review and possibly merge this PR.

Thank you!

@@ -351,14 +351,14 @@ def getHeaders(self, username=None, password=None, headers=None):

if token:
auth_string = b"Bearer " + token.encode("ascii")
headers["Authorization"] = auth_string
headers["Authorization"] = auth_string.decode("ascii")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose ascii as the encoding here to align with the token encoding above.
TBH I am not sure why this is ascii and not utf-8.

Copy link
Member

Choose a reason for hiding this comment

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

ascii should be fine.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR!

@jreadey jreadey merged commit e3ebba4 into HDFGroup:master Feb 7, 2025
8 checks passed
@julienblin-kothar
Copy link
Contributor Author

Awesome - thank you for accepting it!

@julienblin-kothar julienblin-kothar deleted the julienblin-kothar-fix-headers-bytes branch February 7, 2025 13:46
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 this pull request may close these issues.

2 participants