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

fix(azure-key-vault-driver): fix character encoding #308

Merged
merged 2 commits into from Oct 9, 2023

Conversation

itpropro
Copy link
Member

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)

πŸ“š Description

  • Updated key encoding/decoding to avoid decoding errors
  • Updated docs with additional usage hints

@itpropro itpropro requested a review from pi0 September 22, 2023 21:05
@nuxt-studio
Copy link

nuxt-studio bot commented Sep 22, 2023

βœ… Live Preview ready!

Name Edit Preview Latest Commit
unstorage Edit on Studio β†—οΈŽ View Live Preview 916e551

"/": "s",
"=": "-e-",
"+": "-p-",
"/": "-s-",
Copy link
Member

Choose a reason for hiding this comment

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

Would it makes sense / be possible that we use url encoding? (= ~> %3D)

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately not, as Key Vault only supports dashes besides alphanumeric characters. This replacement is only necessary because of the potential characters that could result in a base64 encoding. B64 was the only encoding I found that is available without adding an additional dependency and without requiring a custom implementation of an encoding scheme. As dashes are the only allowed characters and there were too many edge cases with replacing the special characters of b64 with alphanumeric characters or only one dash, this was the least overhead solution.
I can add an additional test of course to validate the encoding.

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Merging #308 (d6364ca) into main (e41a1df) will increase coverage by 0.00%.
Report is 6 commits behind head on main.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #308   +/-   ##
=======================================
  Coverage   77.96%   77.96%           
=======================================
  Files          29       29           
  Lines        3417     3418    +1     
  Branches      521      521           
=======================================
+ Hits         2664     2665    +1     
  Misses        752      752           
  Partials        1        1           
Files Coverage Ξ”
src/drivers/azure-key-vault.ts 36.84% <100.00%> (ΓΈ)

... and 1 file with indirect coverage changes

@pi0 pi0 changed the title fix(azure-key-vault-driver): Fixed encoding/decoding fix(azure-key-vault-driver): fix character encoding Oct 9, 2023
@pi0 pi0 merged commit 0a32d4d into main Oct 9, 2023
5 checks passed
@pi0
Copy link
Member

pi0 commented Oct 9, 2023

Thanks!

@pi0 pi0 deleted the feature/azure-key-vault-fix branch October 9, 2023 11:13
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.

None yet

2 participants