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: regex validate plugin names on plugin add cmd #1010

Merged
merged 9 commits into from Jul 29, 2021
Merged

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

merged 9 commits into from Jul 29, 2021

Conversation

jthegedus
Copy link
Contributor

@jthegedus jthegedus commented Jul 25, 2021

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
test/test_helpers.bash Outdated Show resolved Hide resolved
@Stratus3D
Copy link
Member

Stratus3D commented Jul 29, 2021

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

@jthegedus
Copy link
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
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