Skip to content

Conversation

@kewde
Copy link

@kewde kewde commented Nov 4, 2025

  • Added return after handler.onFailure
  • Added explicit check for Missing relying party ID
  • Added explicit check for non-null data for base64URL encoded data

fixes: https://github.com/ExodusMovement/exodus-mobile/issues/32373

@joshua-rogers-exodus what about Android w.r.t, we don't parse it there, just pass it on to the underlying library, which I assume checks it as well?

  • Added explicit check for Missing relying party ID
  • Added explicit check for non-null data for base64URL encoded data

cc @peterferguson happy to help upstream any of these changes if interested,

@peterferguson
Copy link

@kewde that would be great more defensive programming always welcome!

I am actually working on a change that will throw if the data is not base64url before this point but happy to have checks on both sides 👍

@kewde
Copy link
Author

kewde commented Nov 4, 2025

@peterferguson where would you prevent to have the base64 checks instead?

@joshua-rogers-exodus
Copy link

@joshua-rogers-exodus what about Android w.r.t, we don't parse it there, just pass it on to the underlying library, which I assume checks it as well?

i think it's ok

Copy link

@joshua-rogers-exodus joshua-rogers-exodus left a comment

Choose a reason for hiding this comment

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

utACK

thanks!

@peterferguson
Copy link

@peterferguson where would you prevent to have the base64 checks instead?

I think it is fine where you have it but I am working on updates that make the library compatible with the web api.

You will be able to pass an array buffer directly for those fields. Then on the js side any string passed will be converted from base64url -> buffer. So there will be a defensive guard at that point.

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.

4 participants