Skip to content

path/filepath: EvalSymlinks is broken for relative paths on Windows #16793

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.

Sign up for GitHub

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

Jump to bottom
Closed
luan opened this issue Aug 18, 2016 · 14 comments
Closed

path/filepath: EvalSymlinks is broken for relative paths on Windows #16793

luan opened this issue Aug 18, 2016 · 14 comments
Labels
FrozenDueToAge
Milestone
Go1.7.1

Comments

@luan
Copy link
Contributor

luan commented Aug 18, 2016

This seems to have been introduced with the rework on filepath.EvalSymlinks from this issue: #13980 on this commit: c4dda7e

  1. What version of Go are you using (go version)?
    go version go1.7 windows/amd64
  2. What operating system and processor architecture are you using (go env)?
    windows/amd64
  3. What did you do?

The following code reproduces the problem:

package main

import (
    "fmt"
    "os"
    "path/filepath"
)

func main() {
    path := filepath.Join("..", "..", "foo")
    err := os.MkdirAll(path, 0644)
    if err != nil {
        panic(err)
    }

    evaldPath, err := filepath.EvalSymlinks(path)
    if err != nil {
        panic(err)
    }

    absPath, err := filepath.Abs(evaldPath)
    if err != nil {
        panic(err)
    }

    expectedPath, err := filepath.Abs(path)
    if err != nil {
        panic(err)
    }

    fmt.Printf("original path: %s\n", path)
    fmt.Printf("eval'd path: %s\n", absPath)
    fmt.Printf("expected path: %s\n", expectedPath)
}
  1. What did you expect to see?
> go run main.go
original path: ../../foo
eval'd path: C:\gopath\foo
expected path: C:\gopath\foo
  1. What did you see instead?
> go run main.go
original path: ..\..\foo
eval'd path: C:\gopath\src\pathtest\src\gopath\foo
expected path: C:\gopath\foo
@hirochachacha
Copy link
Contributor

Oh... I've confirmed. Thank you.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/27410 mentions this issue.

@hirochachacha
Copy link
Contributor

FWIW, it doesn't exist before 1.7.
Maybe this is 1.7.1 worthy.

@luan
Copy link
Contributor Author

luan commented Aug 23, 2016

I'd be great if this makes into 1.7.1 since it's a bug introduced on 1.7 and is preventing us from cleanly upgrading to 1.7. And we really really want to upgrade!

@bradfitz bradfitz added this to the Go1.7.1 milestone Aug 23, 2016
@bradfitz
Copy link
Contributor

/cc @ianlancetaylor @adg @broady on 1.7.1 thoughts.

@broady
Copy link
Contributor

broady commented Aug 23, 2016

That looks very broken. How did this break?

@hirochachacha
Copy link
Contributor

Because of my fault. I didn't expect this case. I'm really sorry.

@bradfitz
Copy link
Contributor

@hirochachacha, don't worry about it. We break things all the time. It happens.

@hirochachacha
Copy link
Contributor

thanks.

@ianlancetaylor
Copy link
Member

I'm probably OK with a fix for this going into 1.7.1 but as far as I can tell it has not been fixed on tip yet. The CL attached to this issue has not been submitted, and doesn't have any tests.

@broady
Copy link
Contributor

broady commented Aug 23, 2016

@hirochachacha can you link to the CL that caused this?

@hirochachacha
Copy link
Contributor

https://go-review.googlesource.com/#/c/27412/ is.

@hirochachacha
Copy link
Contributor

Oh, sorry. https://go-review.googlesource.com/#/c/20860/ is original one. (above one is test for this)

@bradfitz
Copy link
Contributor

@hirochachacha please include the fix and new tests in the same CL and have that CL reference this bug ("Fixes #16793")

gopherbot pushed a commit that referenced this issue Sep 7, 2016
@hirochachacha @broady
[release-branch.go1.7] path/filepath: handle ".." in normalizing a pa…
3129c67 
…th on Windows

Current code assumes there are not ".." in the Clean(path).
That's not true. Clean doesn't handle leading "..", so we need to stop
normalization if we see "..".

Fixes #16793

Change-Id: I0a7901bedac17f1210b134d593ebd9f5e8483775
Reviewed-on: https://go-review.googlesource.com/27410
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/28641
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Aug 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.7.1
Development

No branches or pull requests
6 participants
@bradfitz @broady @luan @hirochachacha @ianlancetaylor @gopherbot