x/tools/gopls: panic on filenames containing %

[trapgate]

What did you do?

At some point, someone added a directory to our source tree that contained '%' characters.

This causes gopls to crash (the crash seems to be a recent development, since the directory has been there for about a year.)

You can reproduce this easily, at least on a Mac. I haven't tried this on Linux but I don't see why it wouldn't crash there too. Steps:

1. Create a directory ~/src/%crash_pls%
2. Create a file ~/src/%crash_pls%/main.go
3. Start your editor - I'm using VSCode configured to use gopls, so code ~/src/%crash_pls%
4. Verify that gopls has crashed via notifications and logs.

What did you expect to see?

'%' is a valid character in file paths, so gopls should have worked.

What did you see instead?

The editor reports that gopls has crashed multiple times and won't be restarted.

Stack trace:

goroutine 50 [running]:
golang.org/x/tools/internal/span.URI.Filename(...)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/span/uri.go:28
golang.org/x/tools/internal/lsp/cache.(*view).getGoEnv(0xc00034c000, 0x1a55100, 0xc000299890, 0xc0001a6480, 0x46, 0x46, 0xc0003cce50, 0x100e194, 0xc00033a0a0, 0xa0)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/lsp/cache/view.go:730 +0x77c
golang.org/x/tools/internal/lsp/cache.(*view).setBuildInformation(0xc00034c000, 0x1a55100, 0xc000299890, 0xc0001ca270, 0x29, 0xc0001a6480, 0x46, 0x46, 0x0, 0x0, ...)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/lsp/cache/view.go:622 +0x8f
golang.org/x/tools/internal/lsp/cache.(*session).createView(0xc0000e3b80, 0x1a55100, 0xc000299890, 0xc0002de080, 0xb, 0xc0001ca270, 0x29, 0x4000000000000000, 0x10101, 0xc00016ffd8, ...)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/lsp/cache/session.go:109 +0x4c9
golang.org/x/tools/internal/lsp/cache.(*session).NewView(0xc0000e3b80, 0x1a55100, 0xc000299890, 0xc0002de080, 0xb, 0xc0001ca270, 0x29, 0x4000000000000000, 0x10101, 0xc00016ffd8, ...)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/lsp/cache/session.go:62 +0x13d
golang.org/x/tools/internal/lsp.(*Server).addView(0xc000244a80, 0x1a55100, 0xc000299890, 0xc0002de080, 0xb, 0xc0001ca270, 0x29, 0x8bee37f, 0x1f8a640, 0xc000338000, ...)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/lsp/workspace.go:42 +0x1fe
golang.org/x/tools/internal/lsp.(*Server).addFolders(0xc000244a80, 0x1a55100, 0xc000299890, 0xc00018ec80, 0x1, 0x4)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/lsp/general.go:168 +0x1bc
golang.org/x/tools/internal/lsp.(*Server).initialized(0xc000244a80, 0x1a55100, 0xc000299890, 0x1fa76d0, 0x0, 0x0)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/lsp/general.go:156 +0x263
golang.org/x/tools/internal/lsp.(*Server).Initialized(0xc000244a80, 0x1a55100, 0xc000299890, 0x1fa76d0, 0x1fa76d0, 0x0)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/lsp/server_gen.go:112 +0x49
golang.org/x/tools/internal/lsp/protocol.serverHandler.Deliver(0x1a73100, 0xc000244a80, 0x1a55100, 0xc000299890, 0xc0002b6180, 0xc000299800, 0xc00023e8c0)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/lsp/protocol/tsserver.go:110 +0x970
golang.org/x/tools/internal/jsonrpc2.(*Conn).Run.func1(0xc00003ad20, 0xc0002b6180, 0xc000296780, 0x1a55100, 0xc000299890, 0x0, 0x0, 0xc000247270)
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/jsonrpc2/jsonrpc2.go:370 +0x170
created by golang.org/x/tools/internal/jsonrpc2.(*Conn).Run
/Users/geoffhickey/pkg/mod/golang.org/x/[email protected]/internal/jsonrpc2/jsonrpc2.go:354 +0x877
[Error - 2:34:35 PM] Connection to server got closed. Server will not be restarted.
[Error - 2:34:35 PM] Request textDocument/documentSymbol failed.
Error: Connection got disposed.
at Object.dispose (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/main.js:876:25)
at Object.dispose (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-languageclient/lib/client.js:71:35)
at LanguageClient.handleConnectionClosed (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-languageclient/lib/client.js:2153:42)
at LanguageClient.handleConnectionClosed (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-languageclient/lib/main.js:151:15)
at closeHandler (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-languageclient/lib/client.js:2140:18)
at CallbackList.invoke (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/events.js:62:39)
at Emitter.fire (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/events.js:120:36)
at closeHandler (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/main.js:226:26)
at CallbackList.invoke (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/events.js:62:39)
at Emitter.fire (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/events.js:120:36)
at StreamMessageReader.fireClose (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/messageReader.js:111:27)
at Socket. (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/messageReader.js:151:46)
at Socket.emit (events.js:205:15)
at Pipe. (net.js:586:12)
[Error - 2:34:35 PM] Request textDocument/documentLink failed.
Error: Connection got disposed.
at Object.dispose (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/main.js:876:25)
at Object.dispose (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-languageclient/lib/client.js:71:35)
at LanguageClient.handleConnectionClosed (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-languageclient/lib/client.js:2153:42)
at LanguageClient.handleConnectionClosed (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-languageclient/lib/main.js:151:15)
at closeHandler (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-languageclient/lib/client.js:2140:18)
at CallbackList.invoke (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/events.js:62:39)
at Emitter.fire (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/events.js:120:36)
at closeHandler (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/main.js:226:26)
at CallbackList.invoke (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/events.js:62:39)
at Emitter.fire (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/events.js:120:36)
at StreamMessageReader.fireClose (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/messageReader.js:111:27)
at Socket. (/Users/geoffhickey/.vscode/extensions/ms-vscode.go-0.13.0/node_modules/vscode-jsonrpc/lib/messageReader.js:151:46)
at Socket.emit (events.js:205:15)
at Pipe. (net.js:586:12)

Build info

golang.org/x/tools/gopls v0.3.0
    golang.org/x/tools/[email protected] h1:l9KKK1/n6CIbfgaUvHBWAvCfOxcl1N+KSOK79OlPIao=
    github.com/BurntSushi/[email protected] h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/sergi/[email protected] h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
    golang.org/x/[email protected] h1:WG0RUwxtNT4qqaXX3DPA8zHFNm/D9xaBpxzHt1WcA/E=
    golang.org/x/[email protected] h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
    golang.org/x/[email protected] h1:5ERHXLQfA0b8cHOwaOfWaaGekrA4+Ka/N74zilLnsIk=
    golang.org/x/[email protected] h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
    honnef.co/go/[email protected] h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM=
    mvdan.cc/xurls/[email protected] h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA=

Go info

go version go1.13.6 darwin/amd64

GO111MODULE="off"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/geoffhickey/Library/Caches/go-build"
GOENV="/Users/geoffhickey/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY="*.internal.digitalocean.com,github.com/digitalocean"
GONOSUMDB="*.internal.digitalocean.com,github.com/digitalocean"
GOOS="darwin"
GOPATH="/Users/geoffhickey/do/cthulhu/docode"
GOPRIVATE="*.internal.digitalocean.com,github.com/digitalocean"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13.6/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.6/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/v7/w5lfw5qs1bd9np31z9vv04h80000gn/T/go-build902245116=/tmp/go-build -gno-record-gcc-switches -fno-common"

[gopherbot]

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

[stamblerre]

Thanks for filing this issue - I am able to reproduce and will investigate.

@jayconrod: Does the go command support leading % in filenames? I believe that is invalid.

[jayconrod]

% in file names should be okay. I had to look this up though (in module.CheckFilePath):

CheckFilePath checks that a slash-separated file path is valid. The definition of a valid file path is the same as the definition of a valid import path except that the set of allowed characters is larger: all Unicode letters, ASCII digits, the ASCII space character (U+0020), and the ASCII punctuation characters “!#$%&()+,-.=@[]^_{}~”. (The excluded punctuation characters, " * < > ? ` ' | / \ and :, have special meanings in certain shells or operating systems.)

[gopherbot]

Change https://golang.org/cl/217542 mentions this issue: internal/lsp: attempt handle %s in filepaths

[trapgate]

I did a little more digging. There are some problems with the way filenames are being URL-encoded. I've attached a patch that adds a testcase for this, and some updates to make the test pass. The problems are:

1. In span.NewURI(), the code was calling url.PathUnescape() on the path. This is where the panic was coming from. NewURI is trying to accept both filenames and URIs here, but it's calling PathUnescape before it determines which it has been passed. A filename from the system would never be URL-escaped, so this seems like a bug.

2. There was no place where url.PathEscape() was being called on a filename to turn it into a URI. To do this correctly, I think you have to call PathEscape() on each segment of the path - passing in the whole path will escape the directory separators too, which isn't what you want. I added a loop that does this.

3. A couple of the test cases were expecting to get back URIs with spaces in them. That's not a valid URI - spaces should be escaped as %20.

tools.txt

[trapgate]

Sorry, I've committed a cretinous error and given you a backwards patch. Here's the same patch with the diff params in the right order.

I rebuilt gopls with this patch and fed it to my editor. It does fix the problem, and seems otherwise to be working as expected.

tools.txt

[gopherbot]

Change https://golang.org/cl/217599 mentions this issue: internal/lsp: handle percents by not unescaping URIs

[stamblerre]

Thank you for investigating and for figuring out the issue. We came to a similar conclusion around the same time :) Just mailed the above CL, which should hopefully address the issue.

[gopherbot]

Change https://golang.org/cl/217639 mentions this issue: internal/span, internal/lsp: fix URI escaping

[stamblerre]

This has been cherry-picked onto gopls-release-branch.0.3 and will be in gopls/v0.3.1.

[trapgate]

Thank you for the quick fix! :)