Skip to content
This repository has been archived by the owner on Aug 16, 2024. It is now read-only.

Do not require padding on schema #63

Merged
merged 3 commits into from
Mar 22, 2024
Merged

Do not require padding on schema #63

merged 3 commits into from
Mar 22, 2024

Conversation

orhoj
Copy link
Contributor

@orhoj orhoj commented Mar 20, 2024

Purpose

Allow schema to not have base64 padding.

Changes

  • Allow for no base64 padding in a schema.

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

@orhoj orhoj requested review from abizjak and bisgardo March 20, 2024 14:26
Copy link
Contributor

@bisgardo bisgardo left a comment

Choose a reason for hiding this comment

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

I find it unfortunate that we need to support this. Our tools should only produce base64 with padding (see https://datatracker.ietf.org/doc/html/rfc4648#section-3.2).

I think the leniency of the decoder in JavaScript is the exception, not the norm. So if users try to use the strings in other contexts, it's likely that they'll run into decoding errors and have to add padding anyway.

packages/wallet-connectors/src/WalletConnection.ts Outdated Show resolved Hide resolved
@abizjak
Copy link

abizjak commented Mar 21, 2024

I find it unfortunate that we need to support this. Our tools should only produce base64 with padding (see https://datatracker.ietf.org/doc/html/rfc4648#section-3.2).

I think the leniency of the decoder in JavaScript is the exception, not the norm. So if users try to use the strings in other contexts, it's likely that they'll run into decoding errors and have to add padding anyway.

In many other contexts you have a choice of whether to require padding or not when parsing. For our use of base64 there is no value in padding so it is needlessly disruptive to require it.

It is in fact the choice to arbitrarily require padding that is unfortunate. It has no semantic meaning in our context.

@orhoj orhoj requested a review from bisgardo March 21, 2024 14:44
@orhoj orhoj merged commit 24f7020 into main Mar 22, 2024
1 check passed
@orhoj orhoj deleted the lenient-base64 branch March 22, 2024 09:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants