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

fix resolve link for dirs with trailing / #1113

Merged
merged 1 commit into from
Mar 6, 2020

Conversation

tejal29
Copy link
Contributor

@tejal29 tejal29 commented Mar 6, 2020

Fixes #1112

In this PR,

  • wrap errors wherever possible.
  • Use filepath.Clean to remove trailing '/' from path before recursively finding ancestors.
    filepath.Dir("/foo/bar/baz/") -> returns "foo/bar/baz"
  • Added a unit test with the fix. Without filepath.Clean(path), the test fails
Test_resolveSymlinkAncestor$/^dir_ends_with_$/^_is_not_a_symlink$ #gosetup
=== RUN   Test_resolveSymlinkAncestor
=== RUN   Test_resolveSymlinkAncestor/dir_ends_with_/_is_not_a_symlink
--- FAIL: Test_resolveSymlinkAncestor (0.00s)
    --- FAIL: Test_resolveSymlinkAncestor/dir_ends_with_/_is_not_a_symlink (0.00s)
        resolve_test.go:255: expected result to be /tmp/830246385/var/www not /tmp/830246385/var/www/www
FAIL

See https://play.golang.org/p/TAjN36H796z

package main

import (
	"fmt"
	"path/filepath"
)

func main() {
	fmt.Println("On Unix:")

	fmt.Println(filepath.Dir("/foo/bar/baz"))
	fmt.Println(filepath.Dir("/foo/bar/baz/"))
	fmt.Println(filepath.Dir(filepath.Clean("/foo/bar/baz/")))

}

Returns:

On Unix:
/foo/bar
/foo/bar/baz
/foo/bar

@googlebot googlebot added the cla: yes CLA signed by all commit authors label Mar 6, 2020
@@ -234,6 +235,27 @@ func Test_resolveSymlinkAncestor(t *testing.T) {
}
})

t.Run("dir ends with / is not a symlink", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test for "dir ends with / is a symlink" ?

@tejal29 tejal29 merged commit 861c039 into GoogleContainerTools:master Mar 6, 2020
@tejal29 tejal29 deleted the add_better_error branch April 26, 2021 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA signed by all commit authors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve paths does not resolve correctly for paths with "/"
4 participants