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 66c47f9c4..4b95cc186 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" @@ -185,14 +186,38 @@ 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(rurl.Path)) + 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 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 +} + +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..da86314fe 100644 --- a/mcp/resource_test.go +++ b/mcp/resource_test.go @@ -12,23 +12,37 @@ 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"}, + test{uri: "file://localhost/C:/foo", want: `C:\foo`}, + test{uri: "file://server/share/foo", want: `\\server\share\foo`}, + ) + } 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,10 +61,28 @@ func TestFileRoot(t *testing.T) { } } -func TestComputeURIFilepath(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("TODO: fix for Windows") +func TestFileURIPath(t *testing.T) { + for _, tt := range []struct { + 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"}, + {"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.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) { // TODO(jba): test with Windows \\host paths and C: paths dirFilepath := filepath.FromSlash("/files") rootFilepaths := []string{ @@ -95,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) @@ -136,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 {