close
Skip to content

fix: regex validate plugin names on plugin add cmd#1010

Merged
jthegedus merged 9 commits into
asdf-vm:masterfrom
jthegedus:fix/regex-validate-plugin-names
Jul 29, 2021
Merged

fix: regex validate plugin names on plugin add cmd#1010
jthegedus merged 9 commits into
asdf-vm:masterfrom
jthegedus:fix/regex-validate-plugin-names

Conversation

@jthegedus

@jthegedus jthegedus commented Jul 25, 2021

Copy link
Copy Markdown
Contributor

Summary

Apply regex validation to plugin names when asdf plugin add is used.

Regex is ^[a-zA-Z0-9_-]+$ and applied via grep --extended-regexp.

Fixes: #789

Other Information

^[a-zA-Z0-9-]$ was the original discussion. I have added _ as well as the regex will then cover all popular naming conventions (camelCase, snake_case, kebab-case, PascalCase and UPPER_SNAKE).

We may restrict this regex further in future because of platform independent handling of filesystem paths with varying cases as discussed in asdf-vm/asdf-plugins#28

@jthegedus jthegedus requested a review from a team as a code owner July 25, 2021 05:32
@jthegedus jthegedus changed the title fix: regex validate plugin names fix: regex validate plugin names on add Jul 25, 2021
@jthegedus jthegedus changed the title fix: regex validate plugin names on add fix: regex validate plugin names on plugin add Jul 25, 2021
@jthegedus jthegedus changed the title fix: regex validate plugin names on plugin add fix: regex validate plugin names on plugin add cmd Jul 25, 2021
Comment thread test/test_helpers.bash
@Stratus3D

Stratus3D commented Jul 29, 2021

Copy link
Copy Markdown
Member

Is grep --extended-regexp supported on all platforms? Otherwise changes here look good to me.

@jthegedus

Copy link
Copy Markdown
Contributor Author

I am not sure of the portability of grep --extended-regexp (grep -E, I prefer the clearer extended flag names), but we're already using it in a number of places, so this change doesn't increase the surface area of tools or flags used.

I think we just wait until someone flags grep -E as being an issue for their platform before attempting to remove the dependence.

@jthegedus jthegedus merged commit 7697e6e into asdf-vm:master Jul 29, 2021
@jthegedus jthegedus deleted the fix/regex-validate-plugin-names branch July 29, 2021 22:49
@Stratus3D

Copy link
Copy Markdown
Member

@jthegedus sounds good. I didn't realize core was already using grep -E.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider regex validation of plugin names

2 participants