Escape commas on distinguished names when generating a signining key #373
Labels
Comments
3 tasks
github-merge-queue bot
pushed a commit
to pwa-builder/PWABuilder
that referenced
this issue
Oct 9, 2024
fixes #4791 fixes https://discord.com/channels/930156205542883409/930871438666235964/1292435840554897502 <!-- Link to relevant issue (for ex: "fixes #1234") which will automatically close the issue once the PR is merged --> ## PR Type <!-- Please uncomment one ore more that apply to this PR --> - Bugfix <!-- - Feature --> <!-- - Code style update (formatting) --> <!-- - Refactoring (no functional changes, no api changes) --> <!-- - Build or CI related changes --> <!-- - Documentation content changes --> <!-- - Sample app changes --> <!-- - Other... Please describe: --> ## Describe the current behavior? Depending on the manifest app name or short name it can create an unexpected value for signing key inputs, which causes differentes issues: - The invalid inputs for signing key aren't visible by default (inside of sl-details), even after form submit. - The developer won't know which fields are wrong because the error message is unfriendly. - There is no input validation for these fields. <!-- Please describe the current behavior that is being modified or link to a relevant issue. --> ## Describe the new behavior? After some investigation on bubblewrap, Andre already added some characters escapes here: GoogleChromeLabs/bubblewrap#373 but it's not covering all possible characters that can invalidate the input as coded here: https://github.com/frohoff/jdk8u-jdk/blob/da0da73ab82ed714dc5be94acd2f0d00fbdfe2e9/src/share/classes/sun/security/x509/AVA.java#L95C33-L95C49 We can still open an issue or PR to escape it on bubblewrap, but decided to add a sanitize + validation layer for PWABuilder similar to the existing [input validation for app name](https://github.com/pwa-builder/PWABuilder/blob/0b9255a0eb60e38bc0793b5f761a6fe9c06cdb12/apps/pwabuilder/src/script/utils/constants.ts#L1). (+ Friendly) Behaviors added: - New sanitize function to automatically generate a buildable app at first try. - New dname input validation for the signing key fields to prevent users from typing wrong characters. - On form's submit, if there is any error, it automatically show `sl-details` content allowing to scroll into the error. ## PR Checklist - [x] Test: run `npm run test` and ensure that all tests pass - [x] Target main branch (or an appropriate release branch if appropriate for a bug fix) - [x] Ensure that your contribution follows [standard accessibility guidelines](https://docs.microsoft.com/en-us/microsoft-edge/accessibility/design). Use tools like https://webhint.io/ to validate your changes. ## Additional Information I couldn't find a validate manifest to test the sanitize at first try, the original manifest from the issue is currently down, but here a simple demo of the function:  And demo for the input validation + show sl-details on errors. https://github.com/user-attachments/assets/aba77184-3343-4086-9131-370becc0cd30
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem:
Names are not currently escaped. Using a comma in one of the fields will generate the following command:
which generates the following error:
Fix:
The comma can be escaped like the following:
The text was updated successfully, but these errors were encountered: