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

ConfigPackageUtility::ValidateName: replace broken regex #8825

Merged
merged 1 commit into from Jun 22, 2021

Conversation

julianbrost
Copy link
Contributor

@julianbrost julianbrost commented Jun 7, 2021

The old validation regex matched if the name consists only of invalid character, not that it does not contain them, i.e. something like "foo/bar" was considered valid.

This commit replaces the regex with a check that all characters in the name are allowed characters.

Backwards compatibility

This change could cause trouble in existing installations as package names could have worked before that weren't supposed to be valid. We could also be more permissive then what I suppose was intended with the original implementation.

Before

Running the new tests with the old implementation:

/home/jbrost/dev/icinga2/test/remote-configpackageutility.cpp(21): error: in "remote_configpackageutility/ValidateName": 'foo.bar' should not be valid
/home/jbrost/dev/icinga2/test/remote-configpackageutility.cpp(21): error: in "remote_configpackageutility/ValidateName": 'foo/bar' should not be valid
/home/jbrost/dev/icinga2/test/remote-configpackageutility.cpp(21): error: in "remote_configpackageutility/ValidateName": 'foo:bar' should not be valid

After

New tests pass

@julianbrost julianbrost added the bug Something isn't working label Jun 7, 2021
@julianbrost julianbrost added this to the 2.13.0 milestone Jun 7, 2021
@icinga-probot icinga-probot bot modified the milestones: 2.13.0, 2.14.0 Jun 7, 2021
@julianbrost julianbrost modified the milestones: 2.14.0, 2.13.0 Jun 7, 2021
The old validation regex matched if the name consists only of invalid
character, not that it does not contain them, i.e. something like "foo/bar" was
considered valid.

This commit replaces the regex with a check that all characters in the name are
allowed characters.
@julianbrost julianbrost force-pushed the bugfix/validate-config-package-name branch from 2ea8265 to c40b18e Compare June 15, 2021 10:19
@julianbrost julianbrost requested a review from N-o-X June 15, 2021 10:20
@julianbrost julianbrost merged commit 02f7617 into master Jun 22, 2021
@icinga-probot icinga-probot bot deleted the bugfix/validate-config-package-name branch June 22, 2021 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants