Find extensions whose version directory has no dot in it - #345
Merged
Conversation
load_extension_manifest globbed version directories with "*.*_*", which requires
a dot in the name. An extension unpacked to 7_0 rather than 1.2.3_0 was never
found, so it was reported unreadable with no name, version or permissions, even
though its manifest.json was sitting there intact. Match on the trailing _<n>
instead, since that is the real marker, and keep only directories.
The sort key had a related problem. Splitting the name on '.' leaves the _<n>
suffix attached to the last component, and '3_0' is not a digit string, so it
was compared as text: 1.2.3_0 ranked above 1.2.10_0. Had it been read as a
number it would have been just as wrong, because Python allows underscores in
integer literals and int('3_0') is 30. Split the suffix off first.
magnet.ctf_2018 holds the one single-component version directory in the corpus,
a Google Mail app at 7_0, so its baseline moves: Extensions goes from 3 records
and one unparsed source to 4 and none, and the profile's Extensions status is no
longer partial. Selection is unchanged for the other 54 extensions.
str.isdigit() is true for characters like the superscript '2', which int() then refuses, so a version directory named "²_0" put the ValueError back into the sort key that the surrounding guard exists to prevent. Require ASCII.
Two test cases adopted from dchaudhari7177's independent fix for the same issue in PR #346, which covered ground mine did not. The first is the ordering of single- against multi-component versions. Widening the glob means both shapes can now turn up side by side, so they have to compare against each other and not only among themselves. The second replaces my stray-file test, which used a name that sorted last and so passed with or without the is_dir() filter it was supposed to be pinning. Theirs uses a name that sorts first, which is the case that actually exercises the filter. Reaching the right manifest is not enough on its own even then, since opening a path under a file raises OSError and the loop moves on, so the test also asserts that nothing was logged at error: the right answer by way of a logged error is not the same as the right answer.
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 #344.
load_extension_manifestglobbed version directories with*.*_*, which requires a dot in the name. An extension unpacked to7_0rather than1.2.3_0was never found, so it came out as unreadable with no name, version or permissions even though its manifest.json was right there. It now matches on the trailing_<n>, which is the real marker, and keeps only directories.The sort key had the related problem the issue describes. Splitting the name on
.leaves the_<n>suffix on the last component, and3_0is not a digit string, so it was compared as text and ranked1.2.3_0above1.2.10_0. Reading it as a number would have been just as wrong, since Python allows underscores in integer literals andint('3_0')is 30. The suffix is now split off first.I checked the whole corpus: magnet.ctf_2018 has the only single-component version directory among the 56 there, and it is the only extension whose selected version changes, so "latest version wins" still holds. Its baseline moves accordingly, Extensions from 3 records and one unparsed source to 4 and none, and that profile's Extensions status is no longer partial. Four unit tests added, three of which fail without the fix.
Second commit fixes something adjacent I noticed in the same sort key.
str.isdigit()is true for characters like the superscript '2', whichint()then refuses, so a version directory named²_0would raise straight out of the guard that exists to stop exactly that. It now requires ASCII, with a test that fails without it.