x/tools/gopls: DidOpen (et al?) fail to reject non-absolute path from client early, triggering assertion failure

[adonovan]

#!stacks
"bug.Reportf" && "cache.port.matches:+5"

Issue created by stacks.

func (p port) matches(path string, content []byte) bool {
	ctxt := build.Default // make a copy
	ctxt.UseAllFiles = false
	path = filepath.Clean(path)
	if !filepath.IsAbs(path) {
		bug.Reportf("non-abs file path %q", path)   // <---------
		return false // fail closed
	...

This stack NaAAKw was reported by telemetry:

• gopls/bug
• golang.org/x/tools/gopls/internal/util/bug.report:+35
• golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
• golang.org/x/tools/gopls/internal/cache.port.matches:+5
• golang.org/x/tools/gopls/internal/cache.matchingView[...]:+23
• golang.org/x/tools/gopls/internal/cache.(*Session).viewOfLocked:+15
• golang.org/x/tools/gopls/internal/cache.(*Session).DidModifyFiles:+138
• golang.org/x/tools/gopls/internal/server.(*server).didModifyFiles:+36
• golang.org/x/tools/gopls/internal/server.(*server).DidOpen:+20
• golang.org/x/tools/gopls/internal/protocol.serverDispatch:+253
• golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5
• golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4:+52
• golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1:+2
• golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2:+3
• runtime.goexit:+0

golang.org/x/tools/gopls@v0.17.0-pre.2 go1.23.2 windows/amd64 vscode (2)

Dups: HAiIaQ TeKpaQ odV-aw TO9VqA 4ysk1A iieLfw qlIWfw p47elA sppjQA 1zt-Ww gNtsDQ 8sXekg jAHnFg DwqO8Q Ec_jOw 2F1_QQ V3SuDg

[gabyhelp]

Related Issues

• x/tools/gopls: "unexpected file: %q" bug.Report in bestView (reported by telemetry) #67288
• x/tools/gopls: "inline: corrupted reference %v" bug in golang.RemoveUnusedParameter #69896 (closed)
• x/tools/gopls: "edit does not belong to syntax of package %q" bug in golang.Rename #70049
• x/tools/gopls: bug: load.go:185: go/packages returned unexpected package "golang.org/x/tools/go/callgraph/cha/testdata/iface.go.master" for standalone file #69331 (closed)
• x/tools/gopls: "unexpected files in command-line-arguments package" bug (via telemetry) #64557 (closed)
• x/tools/gopls: ApplyFix: "no file for edit position" bug in suggestedFixToDocumentChange (via telemetry) #68818
• x/tools/gopls: bug in analysis importer reported by telemetry #64236 (closed)
• x/tools/gopls: analysisinternal.ZeroValue crash on (presumably) unsafe.Pointer #70585 (closed)
• x/tools/gopls: toGobDiagnostic bug reported by telemetry #65695 (closed)
• x/tool/cmd/gopls: crashes frequently #32564 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[adonovan]

This implies that session.DidModifyFiles was passed a list of modifications that include relative paths. This is most likely because DidOpen (among others) was passed a relative path that it should have either rejected or absolutized.

[findleyr]

Agreed. The bug is not that the client sent bad paths, but that they weren't rejected earlier.

I feel like there's a whole category of bad inputs that gopls could do a better job of catching and handling. In this case, there's no good way to reject a bad notification, so gopls needs raise a message or diagnostic (or both) alerting the user that their client is misbehaving.

[adonovan]

This stack HAiIaQ was reported by telemetry:

• gopls/bug
• golang.org/x/tools/gopls/internal/util/bug.report:+35
• golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
• golang.org/x/tools/gopls/internal/cache.port.matches:+5
• golang.org/x/tools/gopls/internal/cache.matchingView[...]:+23
• golang.org/x/tools/gopls/internal/cache.(*Session).viewOfLocked:+15
• golang.org/x/tools/gopls/internal/cache.(*Session).SnapshotOf:+3
• golang.org/x/tools/gopls/internal/server.(*server).fileOf:+1
• golang.org/x/tools/gopls/internal/server.(*server).DocumentSymbol:+4
• golang.org/x/tools/gopls/internal/protocol.serverDispatch:+302
• golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5
• golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4:+52
• golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1:+2
• golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2:+3
• runtime.goexit:+0

golang.org/x/tools/gopls@v0.17.1 go1.23.3 windows/amd64 other,vscode (1)

[adonovan]

This stack TeKpaQ was reported by telemetry:

• gopls/bug
• golang.org/x/tools/gopls/internal/util/bug.report:+35
• golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
• golang.org/x/tools/gopls/internal/cache.port.matches:+5
• golang.org/x/tools/gopls/internal/cache.matchingView[...]:+23
• golang.org/x/tools/gopls/internal/cache.selectViewDefs:+59
• golang.org/x/tools/gopls/internal/cache.(*Session).DidModifyFiles:+93
• golang.org/x/tools/gopls/internal/server.(*server).didModifyFiles:+36
• golang.org/x/tools/gopls/internal/server.(*server).DidOpen:+20
• golang.org/x/tools/gopls/internal/protocol.serverDispatch:+253
• golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5
• golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4:+52
• golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1:+2
• golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2:+3
• runtime.goexit:+0

golang.org/x/tools/gopls@v0.17.1 go1.23.4 windows/amd64 vscode (1)