Repository navigation
fix(sourcereader): exclude single-character URL schemes in Windows drive detection - #6380
Conversation
…ive detection Change-Id: I46694e4e2739babce473855669e8e71f7fc9b91b
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the reliability of Windows drive-letter path detection within the source reader. By refining the classification logic and removing reliance on platform-specific standard library functions, the changes ensure that remote URIs and local paths are correctly distinguished across all supported environments, addressing potential misclassifications in previous implementations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the isWindowsDrivePath function in pkg/sourcereader/sourcereader.go to accept a normalized cleanSource path and adds robust checks to handle edge cases such as single-character URL schemes, forced protocol prefixes, and unnormalized multi-slash local paths. It also removes the unused path/filepath import and updates IsLocalPath accordingly. Additionally, comprehensive unit tests have been added to pkg/sourcereader/sourcereader_test.go to validate these changes against various local and remote path patterns. As there are no review comments, I have no additional feedback to provide.
dcb7048
into
GoogleCloudPlatform:develop
Summary
Follow-up to #6325 addressing review feedback regarding Windows drive-letter path detection (
isWindowsDrivePath).Previously,
isWindowsDrivePathmatched any string starting with[A-Za-z]:, andIsLocalPathalso fell back tofilepath.IsAbs(source). This had two subtle edge cases:s://bucket/modules/vpc) orgo-getterforced protocol prefixes (e.g.,s::https://...) would be misclassified as local Windows drive paths.filepath.IsAbs("s://bucket/modules/vpc")returnstrue(treatings:as a volume name and/as a path separator), bypassing scheme checks on Windows runners.Changes
pkg/sourcereader/sourcereader.go:isWindowsDrivePath(cleanSource)to explicitly rejectgo-getterforced protocol syntax (cleanSource[2] == ':') and URI authority prefixes (strings.HasPrefix(cleanSource[2:], "//")).cleanSourcefromIsLocalPathintoisWindowsDrivePathto avoid redundant string allocations.filepath.IsAbs(source)fromIsLocalPathso local vs. remote path classification is 100% deterministic across Linux, macOS, and Windows (POSIX/, Windows UNC\\server\share, and drive pathsC:\,C:/,C:pathare all handled directly viacleanSourceandisWindowsDrivePath).C://foo) and single-character opaque URIs (s:manifest.json).pkg/sourcereader/sourcereader_test.go:TestIsLocalPath,TestIsRemotePath, andTestFactoryfor drive roots (C:,C:\,C:/), UNC paths (\\server\share,//server/share), single-character schemes (s://,c://,s::), and boundary conditions.Verification
./pkg/sourcereader/...,./pkg/shell/...,./pkg/modulereader/...) on both Linux and native Windows Server 2022 (go1.26.8 windows/amd64).gcluster.exe createandterraform validateon Windows Server 2022.