Refactor & make base64 functions browser-safe #3818
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We had two identical sets of base64 functions in the js-sdk, both using Buffer which isn't really available in the browser unless you're using an old webpack (ie. what element-web uses). This PR: * Takes the crypto base64 file and moves it out of crypto (because we use base64 for much more than just crypto) * Makes them work in a browser without the Buffer global * Removes the other base64 functions * Changes everything to use the new common ones * Adds a comment explaining why the function is kinda ugly and how soul destroyingly awful the JS ecosystem is. * Runs the tests with both impls * Changes the test to not just test the decoder against the encoder * Adds explicit support & tests for (decoding) base64Url (I'll add an encode method later, no need for that to go in this PR too).
|
Sonarcloud is unhappy because some of the lines where I've changed the import are untested. I don't think it's really appropriate for this to extend further into a mission writing tests for the crypto code. |
t3chguy
reviewed
Oct 20, 2023
t3chguy
approved these changes
Oct 20, 2023
3 tasks
richvdh
added
X-Breaking-Change
T-Deprecation
|
giving this an X-Breaking-Change label as it did actually break the react sdk |
dbkr
added a commit
to matrix-org/matrix-react-sdk
that referenced
this pull request
Oct 20, 2023
Apologies, I broke this with matrix-org/matrix-js-sdk#3818 This fixes it, but needs matrix-org/matrix-js-sdk#3820
3 tasks
dbkr
added a commit
to matrix-org/matrix-react-sdk
that referenced
this pull request
Oct 20, 2023
Apologies, I broke this with matrix-org/matrix-js-sdk#3818 This fixes it, but needs matrix-org/matrix-js-sdk#3820
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We had two identical sets of base64 functions in the js-sdk, both using Buffer which isn't really available in the browser unless you're using an old webpack (ie. what element-web uses). This PR:
A fair amount, but most of it is just updating refs to the relocated file, so hopefully readily digestible.
Checklist
Here's what your changelog entry will look like:
🚨 BREAKING CHANGES