9fans / acme-lsp

Language Server Protocol tools for the acme text editor
MIT License
193 stars 25 forks source link

In internal/lsp/text/edit.go, ToPath doesn't percent decode input URI #60

Open alurm opened 12 months ago

alurm commented 12 months ago

Here's an example.

On my machine, C++'s std::deque is located at this path:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/deque

I tried doing "L def" on std::deque.

Since the path contains ++ ("c++"), when it got to Acme, it was interpreted as this filename:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c%2B%2B/v1/deque

Notice percent encoded %2B%2B.

Since this file doesn't exist, this error is printed:

can't open /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c%2B%2B/v1/deque: No such file or directory

alurm commented 12 months ago

Here is a diff to quickly fix this:

diff --git a/internal/lsp/text/edit.go b/internal/lsp/text/edit.go
index 3848358..177f87b 100644
--- a/internal/lsp/text/edit.go
+++ b/internal/lsp/text/edit.go
@@ -4,6 +4,7 @@ package text
 import (
    "fmt"
    "io"
+   "net/url"
    "sort"
    "strings"

@@ -127,6 +128,7 @@ func ToURI(filename string) protocol.DocumentURI {
 // ToPath converts URI to filename.
 func ToPath(uri protocol.DocumentURI) string {
    filename, _ := CutPrefix(string(uri), "file://")
+   filename, _ = url.PathUnescape(filename)
    return filename
 }
alurm commented 12 months ago

It may be the case that something about ToURI should be done as well.

It currently doesn't decode escape the path.

I tried defining ToURI as such:

// ToURI converts filename to URI.
func ToURI(filename string) protocol.DocumentURI {
    return protocol.DocumentURI("file://" + url.PathEscape(filename))
}

This doesn't work. I think it's because url.PathEscape escapes "/" as well.