From d85cdaa979dd3e5ab11bf1f2bacc1191b8dbe6e7 Mon Sep 17 00:00:00 2001 From: Akshita <110122283+akshita317@users.noreply.github.com> Date: Sat, 24 Oct 2026 01:27:22 +0530 Subject: [PATCH 1/2] mcp: accept Windows drive letter file roots fileRoot turns the client's roots into absolute file paths for FileResourceHandler. On Windows a root for a drive letter path is a URI such as file:///C:/Users/me/project (RFC 8089, Appendix E.2), whose path is /C:/Users/me/project. fileRoot converted that to \C:\Users\me\project, which is not an absolute path on Windows, so every such root failed with "not an absolute path", and with it every read through FileResourceHandler once the client had any roots. Drop the slash before a drive letter on Windows. VS Code's form, file:///c%3A/..., decodes to the same path. TestFileRoot is no longer skipped on Windows and gets drive letter cases there; they fail without the change. TestFileURIPath covers the conversion for both Windows and other systems on any platform. --- mcp/resource.go | 18 ++++++++++++++- mcp/resource_test.go | 52 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/mcp/resource.go b/mcp/resource.go index 66c47f9c4..aece8262d 100644 --- a/mcp/resource.go +++ b/mcp/resource.go @@ -13,6 +13,7 @@ import ( "net/url" "os" "path/filepath" + "runtime" "strings" "github.com/modelcontextprotocol/go-sdk/internal/mcpgodebug" @@ -186,13 +187,28 @@ func fileRoot(root *Root) (_ string, err error) { return "", errors.New("empty path") } // We don't want Localize here: we want an absolute path, which is not local. - fileRoot := filepath.Clean(filepath.FromSlash(rurl.Path)) + fileRoot := filepath.Clean(filepath.FromSlash(fileURIPath(rurl.Path, runtime.GOOS))) if !filepath.IsAbs(fileRoot) { return "", errors.New("not an absolute path") } return fileRoot, nil } +// fileURIPath returns the path of a file URI in the form used by goos. +// On Windows, the path of a file URI for a drive letter path starts with a +// slash, as in file:///C:/dir (RFC 8089, Appendix E.2), which is dropped so +// that the result is the absolute path C:/dir. +func fileURIPath(uriPath, goos string) string { + if goos == "windows" && len(uriPath) >= 3 && uriPath[0] == '/' && uriPath[2] == ':' && isASCIILetter(uriPath[1]) { + return uriPath[1:] + } + return uriPath +} + +func isASCIILetter(c byte) bool { + return 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' +} + // Matches reports whether the receiver's uri template matches the uri. func (sr *serverResourceTemplate) Matches(uri string) bool { tmpl, err := uritemplate.New(sr.resourceTemplate.URITemplate) diff --git a/mcp/resource_test.go b/mcp/resource_test.go index 36ed3f1a4..8ced53a5d 100644 --- a/mcp/resource_test.go +++ b/mcp/resource_test.go @@ -12,23 +12,35 @@ import ( ) func TestFileRoot(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("TODO: fix for Windows") - } - - for _, tt := range []struct { + type test struct { uri string want string wantErr string // error must contain this string - }{ - {uri: "file:///foo", want: "/foo"}, - {uri: "file:///foo/bar", want: "/foo/bar"}, - {uri: "file:///foo/../bar", want: "/bar"}, - {uri: "file:/foo", want: "/foo"}, + } + tests := []test{ {uri: "http:///foo", wantErr: "not a file"}, {uri: "file://foo", wantErr: "empty path"}, {uri: ":", wantErr: "missing protocol scheme"}, - } { + } + if runtime.GOOS == "windows" { + tests = append(tests, + test{uri: "file:///C:/foo", want: `C:\foo`}, + test{uri: "file:///c:/foo/bar", want: `c:\foo\bar`}, + test{uri: "file:///C:/foo/../bar", want: `C:\bar`}, + test{uri: "file:/C:/foo", want: `C:\foo`}, + // VS Code percent-encodes the colon after the drive letter. + test{uri: "file:///c%3A/foo", want: `c:\foo`}, + test{uri: "file:///foo", wantErr: "not an absolute path"}, + ) + } else { + tests = append(tests, + test{uri: "file:///foo", want: "/foo"}, + test{uri: "file:///foo/bar", want: "/foo/bar"}, + test{uri: "file:///foo/../bar", want: "/bar"}, + test{uri: "file:/foo", want: "/foo"}, + ) + } + for _, tt := range tests { got, err := fileRoot(&Root{URI: tt.uri}) if err != nil { if tt.wantErr == "" { @@ -47,6 +59,24 @@ func TestFileRoot(t *testing.T) { } } +func TestFileURIPath(t *testing.T) { + for _, tt := range []struct { + uriPath, goos, want string + }{ + {"/C:/foo", "windows", "C:/foo"}, + {"/c:/foo", "windows", "c:/foo"}, + {"/C:", "windows", "C:"}, + {"/foo", "windows", "/foo"}, + {"/1:/foo", "windows", "/1:/foo"}, + {"/C:/foo", "linux", "/C:/foo"}, + {"/foo", "linux", "/foo"}, + } { + if got := fileURIPath(tt.uriPath, tt.goos); got != tt.want { + t.Errorf("fileURIPath(%q, %q) = %q, want %q", tt.uriPath, tt.goos, got, tt.want) + } + } +} + func TestComputeURIFilepath(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("TODO: fix for Windows") From 16a042b44a8d80efb60aa3f041908454857b8e26 Mon Sep 17 00:00:00 2001 From: Akshita <110122283+akshita317@users.noreply.github.com> Date: Tue, 10 Nov 2026 00:22:37 +0530 Subject: [PATCH 2/2] mcp: accept UNC file roots on Windows and unskip the Windows tests A file URI with a host other than localhost names a UNC path on Windows (RFC 8089, Appendix E.3.1), so file://server/share/dir is now the root \server\share\dir rather than a path that drops the host. With file roots working on Windows, the resource tests skipped there pass, so remove the skips. TestEndToEnd built its root as "file://" followed by the path, which on Windows is not a file URI (file://C:\dir); build it with net/url instead. Drop the stale comment about Localize in fileRoot. --- mcp/mcp_test.go | 119 +++++++++++++++++++++---------------------- mcp/resource.go | 25 ++++++--- mcp/resource_test.go | 34 ++++++------- 3 files changed, 91 insertions(+), 87 deletions(-) diff --git a/mcp/mcp_test.go b/mcp/mcp_test.go index b15cd7fb0..76fc9721e 100644 --- a/mcp/mcp_test.go +++ b/mcp/mcp_test.go @@ -18,7 +18,6 @@ import ( "net/http/httptest" "net/url" "path/filepath" - "runtime" "slices" "strings" "sync" @@ -168,7 +167,10 @@ func TestEndToEnd(t *testing.T) { if err != nil { t.Fatal(err) } - c.AddRoots(&Root{URI: "file://" + rootAbs}) + // A file URI path starts with a slash, which a Windows path such as + // C:\dir does not. + rootURI := &url.URL{Scheme: "file", Path: "/" + strings.TrimPrefix(filepath.ToSlash(rootAbs), "/")} + c.AddRoots(&Root{URI: rootURI.String()}) // Connect the client. // @@ -295,73 +297,70 @@ func TestEndToEnd(t *testing.T) { // ===== resources ===== t.Log("Testing resources") - //TODO: fix for Windows - if runtime.GOOS != "windows" { - wantResources := []*Resource{resource2, resource1} - lrres, err := cs.ListResources(ctx, nil) - if err != nil { - t.Fatal(err) - } - if diff := cmp.Diff(wantResources, lrres.Resources); diff != "" { - t.Errorf("resources/list mismatch (-want, +got):\n%s", diff) - } + wantResources := []*Resource{resource2, resource1} + lrres, err := cs.ListResources(ctx, nil) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(wantResources, lrres.Resources); diff != "" { + t.Errorf("resources/list mismatch (-want, +got):\n%s", diff) + } - template := &ResourceTemplate{ - Name: "rt", - MIMEType: "text/template", - URITemplate: "file:///{+filename}", // the '+' means that filename can contain '/' - } - s.AddResourceTemplate(template, readHandler) - tres, err := cs.ListResourceTemplates(ctx, nil) - if err != nil { - t.Fatal(err) - } - if diff := cmp.Diff([]*ResourceTemplate{template}, tres.ResourceTemplates); diff != "" { - t.Errorf("resources/list mismatch (-want, +got):\n%s", diff) - } + template := &ResourceTemplate{ + Name: "rt", + MIMEType: "text/template", + URITemplate: "file:///{+filename}", // the '+' means that filename can contain '/' + } + s.AddResourceTemplate(template, readHandler) + tres, err := cs.ListResourceTemplates(ctx, nil) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff([]*ResourceTemplate{template}, tres.ResourceTemplates); diff != "" { + t.Errorf("resources/list mismatch (-want, +got):\n%s", diff) + } - for _, tt := range []struct { - uri string - mimeType string // "": not found; "text/plain": resource; "text/template": template - fail bool // non-nil error returned - }{ - {"file:///info.txt", "text/plain", false}, - {"file:///fail.txt", "", false}, - {"file:///template.txt", "text/template", false}, - {"file:///../private.txt", "", true}, // not found: escaping disallowed - } { - rres, err := cs.ReadResource(ctx, &ReadResourceParams{URI: tt.uri}) - if err != nil { - if code := errorCode(err); code == CodeResourceNotFound { - if tt.mimeType != "" { - t.Errorf("%s: not found but expected it to be", tt.uri) - } - } else if !tt.fail { - t.Errorf("%s: unexpected error %v", tt.uri, err) + for _, tt := range []struct { + uri string + mimeType string // "": not found; "text/plain": resource; "text/template": template + fail bool // non-nil error returned + }{ + {"file:///info.txt", "text/plain", false}, + {"file:///fail.txt", "", false}, + {"file:///template.txt", "text/template", false}, + {"file:///../private.txt", "", true}, // not found: escaping disallowed + } { + rres, err := cs.ReadResource(ctx, &ReadResourceParams{URI: tt.uri}) + if err != nil { + if code := errorCode(err); code == CodeResourceNotFound { + if tt.mimeType != "" { + t.Errorf("%s: not found but expected it to be", tt.uri) } + } else if !tt.fail { + t.Errorf("%s: unexpected error %v", tt.uri, err) + } + } else { + if tt.fail { + t.Errorf("%s: unexpected success", tt.uri) + } else if g, w := len(rres.Contents), 1; g != w { + t.Errorf("got %d contents, wanted %d", g, w) } else { - if tt.fail { - t.Errorf("%s: unexpected success", tt.uri) - } else if g, w := len(rres.Contents), 1; g != w { - t.Errorf("got %d contents, wanted %d", g, w) - } else { - c := rres.Contents[0] - if got := c.URI; got != tt.uri { - t.Errorf("got uri %q, want %q", got, tt.uri) - } - if got := c.MIMEType; got != tt.mimeType { - t.Errorf("%s: got MIME type %q, want %q", tt.uri, got, tt.mimeType) - } + c := rres.Contents[0] + if got := c.URI; got != tt.uri { + t.Errorf("got uri %q, want %q", got, tt.uri) + } + if got := c.MIMEType; got != tt.mimeType { + t.Errorf("%s: got MIME type %q, want %q", tt.uri, got, tt.mimeType) } } } - - s.AddResource(&Resource{URI: "http://U"}, nil) - waitForNotification(t, "resources") - s.RemoveResources("http://U") - waitForNotification(t, "resources") } + s.AddResource(&Resource{URI: "http://U"}, nil) + waitForNotification(t, "resources") + s.RemoveResources("http://U") + waitForNotification(t, "resources") + // ===== roots ===== t.Log("Testing roots") { diff --git a/mcp/resource.go b/mcp/resource.go index aece8262d..4b95cc186 100644 --- a/mcp/resource.go +++ b/mcp/resource.go @@ -186,20 +186,29 @@ func fileRoot(root *Root) (_ string, err error) { // common mistake "file://foo". return "", errors.New("empty path") } - // We don't want Localize here: we want an absolute path, which is not local. - fileRoot := filepath.Clean(filepath.FromSlash(fileURIPath(rurl.Path, runtime.GOOS))) + fileRoot := filepath.Clean(filepath.FromSlash(fileURIPath(rurl.Host, rurl.Path, runtime.GOOS))) if !filepath.IsAbs(fileRoot) { return "", errors.New("not an absolute path") } return fileRoot, nil } -// fileURIPath returns the path of a file URI in the form used by goos. -// On Windows, the path of a file URI for a drive letter path starts with a -// slash, as in file:///C:/dir (RFC 8089, Appendix E.2), which is dropped so -// that the result is the absolute path C:/dir. -func fileURIPath(uriPath, goos string) string { - if goos == "windows" && len(uriPath) >= 3 && uriPath[0] == '/' && uriPath[2] == ':' && isASCIILetter(uriPath[1]) { +// fileURIPath returns the path of a file URI with the given host and path in +// the form used by goos. On Windows: +// - The path of a file URI for a drive letter path starts with a slash, as +// in file:///C:/dir (RFC 8089, Appendix E.2), which is dropped so that the +// result is the absolute path C:/dir. +// - A file URI with a host other than localhost names a UNC path, as in +// file://server/share/dir (RFC 8089, Appendix E.3.1), whose result is +// //server/share/dir. +func fileURIPath(host, uriPath, goos string) string { + if goos != "windows" { + return uriPath + } + if host != "" && !strings.EqualFold(host, "localhost") { + return "//" + host + uriPath + } + if len(uriPath) >= 3 && uriPath[0] == '/' && uriPath[2] == ':' && isASCIILetter(uriPath[1]) { return uriPath[1:] } return uriPath diff --git a/mcp/resource_test.go b/mcp/resource_test.go index 8ced53a5d..da86314fe 100644 --- a/mcp/resource_test.go +++ b/mcp/resource_test.go @@ -31,6 +31,8 @@ func TestFileRoot(t *testing.T) { // VS Code percent-encodes the colon after the drive letter. test{uri: "file:///c%3A/foo", want: `c:\foo`}, test{uri: "file:///foo", wantErr: "not an absolute path"}, + test{uri: "file://localhost/C:/foo", want: `C:\foo`}, + test{uri: "file://server/share/foo", want: `\\server\share\foo`}, ) } else { tests = append(tests, @@ -61,26 +63,26 @@ func TestFileRoot(t *testing.T) { func TestFileURIPath(t *testing.T) { for _, tt := range []struct { - uriPath, goos, want string + host, uriPath, goos, want string }{ - {"/C:/foo", "windows", "C:/foo"}, - {"/c:/foo", "windows", "c:/foo"}, - {"/C:", "windows", "C:"}, - {"/foo", "windows", "/foo"}, - {"/1:/foo", "windows", "/1:/foo"}, - {"/C:/foo", "linux", "/C:/foo"}, - {"/foo", "linux", "/foo"}, + {"", "/C:/foo", "windows", "C:/foo"}, + {"", "/c:/foo", "windows", "c:/foo"}, + {"", "/C:", "windows", "C:"}, + {"", "/foo", "windows", "/foo"}, + {"", "/1:/foo", "windows", "/1:/foo"}, + {"localhost", "/C:/foo", "windows", "C:/foo"}, + {"server", "/share/foo", "windows", "//server/share/foo"}, + {"", "/C:/foo", "linux", "/C:/foo"}, + {"", "/foo", "linux", "/foo"}, + {"server", "/share/foo", "linux", "/share/foo"}, } { - if got := fileURIPath(tt.uriPath, tt.goos); got != tt.want { - t.Errorf("fileURIPath(%q, %q) = %q, want %q", tt.uriPath, tt.goos, got, tt.want) + if got := fileURIPath(tt.host, tt.uriPath, tt.goos); got != tt.want { + t.Errorf("fileURIPath(%q, %q, %q) = %q, want %q", tt.host, tt.uriPath, tt.goos, got, tt.want) } } } func TestComputeURIFilepath(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("TODO: fix for Windows") - } // TODO(jba): test with Windows \\host paths and C: paths dirFilepath := filepath.FromSlash("/files") rootFilepaths := []string{ @@ -125,9 +127,6 @@ func TestComputeURIFilepath(t *testing.T) { } func TestReadFileResource(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("TODO: fix for Windows") - } abs, err := filepath.Abs("testdata") if err != nil { t.Fatal(err) @@ -166,9 +165,6 @@ func TestTemplateMatch(t *testing.T) { // error. Regression: the message formatted filepath.Localize's error-path // return value, which is always "", so it never identified the offending path. func TestComputeURIFilepathLocalizeError(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("TODO: fix for Windows") - } dirFilepath := filepath.FromSlash("/files") _, err := computeURIFilepath("file://foo/../bar", dirFilepath, nil) if err == nil {