Fix cwk_path_get_extension treating . and .. as having an extension - #59
Open
afonsojanu wants to merge 1 commit into
Open
afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
cwk_path_get_extension() scans the last path segment for a dot without checking whether that segment is actually a filename. For the special current and back segments, which are made up entirely of dots, this means the function reports an extension of "." even though those segments are directory references and never have an extension at all. cwk_path_get_basename() already treats these segments as opaque wholes (there is a test covering that), so get_extension should follow the same rule. The fix skips the dot scan whenever the segment type is not CWK_NORMAL, which cwk_path_get_segment_type already tells us. Added extension_get_special_directories to the test suite, which checks cwk_path_get_extension and cwk_path_has_extension against a bare ".", a bare "..", and both nested inside a longer path.
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.
cwk_path_get_extension finds the last segment of a path and then scans it backwards for a dot, treating everything from that dot to the end of the segment as the extension. That scan doesn't look at what kind of segment it's looking at, so it also runs on the current and back segments (
.and..), which are made up entirely of dots but represent directory references rather than filenames.The practical effect is that
cwk_path_get_extension("..")returns true with an extension of".", and the same happens for a path like/some/folder/...cwk_path_has_extensioninherits the same issue since it just wrapsget_extension. I ran into this while filtering directory entries by extension and got surprised that..showed up as having one.This is inconsistent with the rest of the library.
cwk_path_get_basenamealready special-cases these segments and returns them as opaque names (there's abasename_special_directoriestest for that), andcwk_path_get_segment_typealready exists specifically to tell current/back segments apart from normal ones. The fix just checks the segment type before scanning for a dot, and skips the scan entirely unless the segment isCWK_NORMAL.I added an
extension_get_special_directoriestest that checkscwk_path_get_extensionandcwk_path_has_extensionagainst a bare., a bare.., and both nested at the end of a longer path. I verified it fails on the current code and passes with the fix, and ran the full existing suite (180 tests before this change, 181 after) under ASan/UBSan with no failures or regressions.I left the unrelated case of a path like
...alone, since that already goes through the same simple last-dot scan today and isn't a directory-reference segment, so it seemed out of scope for this fix.