Reject empty strings in Maven, Nuget, Rubygems and Conan versions - #209
Open
MGpromax wants to merge 1 commit into
Open
Reject empty strings in Maven, Nuget, Rubygems and Conan versions#209MGpromax wants to merge 1 commit into
MGpromax wants to merge 1 commit into
Conversation
MavenVersion, NugetVersion, RubygemsVersion and ConanVersion accepted the empty string because their is_valid() overrides only tried to parse the value, and the underlying parsers accept empty input. An accepted empty version then sorted below every real version (Maven, Rubygems), raised TypeError on comparison (Nuget), or both (Conan), silently corrupting downstream range logic instead of failing. Make the overrides honor the base Version.is_valid() contract, which already documents that the empty string is invalid, by checking super().is_valid() first. DebianVersion gets the same guard for consistency although its parser already rejected empty input. Fixes aboutcode-org#204 Signed-off-by: Manoj Gowda <manojgowdabs18@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #204
MavenVersion,NugetVersion,RubygemsVersion-- and alsoConanVersion, which the issue did not list but has the same hole (found by auditing everyVersionsubclass with an empty string) -- accepted""because theiris_valid()overrides only try to parse the value and the underlying parsers accept empty input. The baseVersion.is_valid()docstring already promises that the empty string is invalid; the overrides just never consulted it.The fix makes each override check
super().is_valid(string)first, so all four now raiseInvalidVersion("")likeSemverVersionandPypiVersiondo, instead of the three different silent behaviors described in the issue (sorts-as-minimum, TypeError on comparison, or both).DebianVersionshares the same try/except body and gets the same guard for consistency, although its parser already rejected empty input.Added a regression test covering the fixed classes plus
SemverVersion/PypiVersionas canaries.Test suite:
7272 passed; the 2 failures intest_semver_version/test_enhanced_semantic_version(semver build-metadata ordering) are pre-existing on a clean checkout with the same environment and unrelated to this change.