diff --git a/services/graph/pkg/middleware/path_lookup.go b/services/graph/pkg/middleware/path_lookup.go index 7ddb0cc7c8..695e8b988b 100644 --- a/services/graph/pkg/middleware/path_lookup.go +++ b/services/graph/pkg/middleware/path_lookup.go @@ -231,38 +231,53 @@ func rewriteColonPath( // // Below the drive the grammar is one of: // -// /root: -// /items/: +// /root:/[:/][:] +// /items/:/[:/][:] // -// where is "/", optionally followed by ":/", and an -// optional trailing ":". Colons are structural delimiters, so neither -// nor contains one. For example: +// The structural delimiter is ":/" - a colon immediately followed by the +// leading slash of the path or suffix - not a bare ":". Splitting on ":/" +// keeps a ':' *inside* a file or directory name as data instead of mistaking +// it for a delimiter (segments are '/'-separated and never start with ':'). +// For example: // // /root:/Documents -> path "/Documents" // /root:/Documents: -> path "/Documents" // /root:/Documents:/children -> path "/Documents", suffix "/children" +// /root:/a:b.txt -> path "/a:b.txt" (colon kept in the name) // /items/{id}:/notes.txt:/children -> itemID "{id}", path "/notes.txt", suffix "/children" // +// A raw ':' at a segment boundary is ambiguous - "/root:/foo:/bar" reads as +// path "/foo" with suffix "/bar", and a trailing ':' is the no-suffix +// terminator. To address a name whose ':' sits at a boundary (e.g. a name +// ending in ':'), percent-encode it as "%3A": the split works on the literal +// ":/", so "%3A" is never a delimiter and decodes back to ':' with the rest of +// the path (e.g. "/root:/weird%3A/file.txt" -> "/weird:/file.txt"). MS Graph +// sidesteps this by forbidding ':' in names entirely; OpenCloud allows it (via +// WebDAV), so "%3A" is how such names are reached here. +// // The returned fields are the raw (still percent-encoded) substrings; the // caller decodes them. func parseColonPath(routePath string) (colonMatch, bool) { var m colonMatch - // The anchor is separated from the rest by the first colon; no colon at all + // The anchor is separated from the path by the first ":/"; no ":/" at all // means this isn't colon syntax and should pass through. - anchor, rest, found := strings.Cut(routePath, ":") + anchor, rest, found := strings.Cut(routePath, ":/") if !found { return m, false } + // Cut consumed the leading '/' of the path along with the ":/" delimiter; + // the path is absolute, so put it back. + rest = "/" + rest switch { case anchor == "/root": // Root-anchored: path resolution later anchors on the {driveID} route - // param, so there's no item id to capture from the URL. + // param, so there's no item id to read from the URL. case strings.HasPrefix(anchor, "/items/"): // Item-anchored: the anchor is /items/{itemID} with a single-segment // id. A '/' inside the id means this is an ordinary /items/{id}/... - // request that just happens to contain a colon further along. + // request that just happens to contain a ":/" further along. itemID := strings.TrimPrefix(anchor, "/items/") if itemID == "" || strings.Contains(itemID, "/") { return m, false @@ -273,21 +288,24 @@ func parseColonPath(routePath string) (colonMatch, bool) { return m, false } - // rest is "/[:/][:]". Drop a single optional trailing ':' - // (the "...:" no-suffix shape), then split path from an optional suffix on - // the one remaining delimiter colon. strings.Cut leaves the suffix empty - // when there's no delimiter: a suffix requires an explicit second colon, - // so e.g. "/root:/foo/children" is the path "/foo/children", not the path - // "/foo" with suffix "/children". - rest = strings.TrimSuffix(rest, ":") - m.relPath, m.suffix, _ = strings.Cut(rest, ":") + // rest is "/[:/][:]". Split path from an optional suffix on + // the next ":/" (the suffix regains the leading '/' Cut consumed), then + // drop a single trailing ':' - the "...:" no-suffix terminator. A suffix + // therefore requires an explicit ":/", so "/root:/foo/children" is the path + // "/foo/children", not "/foo" with suffix "/children". + m.relPath = rest + if path, suffix, found := strings.Cut(rest, ":/"); found { + m.relPath, m.suffix = path, "/"+suffix + } + m.relPath = strings.TrimSuffix(m.relPath, ":") // Shape requirements: the path must be absolute and non-empty ("/x"), and a - // present suffix must be absolute and colon-free. + // present suffix must be absolute, non-empty and colon-free (suffixes are + // fixed API routes like "/children", never file paths). if len(m.relPath) < 2 || m.relPath[0] != '/' { return m, false } - if m.suffix != "" && (m.suffix[0] != '/' || strings.Contains(m.suffix, ":")) { + if m.suffix != "" && (len(m.suffix) < 2 || strings.Contains(m.suffix, ":")) { return m, false } return m, true diff --git a/services/graph/pkg/middleware/path_lookup_test.go b/services/graph/pkg/middleware/path_lookup_test.go index 9347862c03..ee36f1d41b 100644 --- a/services/graph/pkg/middleware/path_lookup_test.go +++ b/services/graph/pkg/middleware/path_lookup_test.go @@ -163,7 +163,7 @@ func TestResolveGraphPath(t *testing.T) { // Anchoring: the colon-syntax shape appearing under a junk prefix // (i.e. NOT at the configured HTTP root) must not trigger a rewrite // or a CS3 Stat. Because the middleware is mounted under the - // /graph root, chi never routes such a path into it (404 first) — + // /graph root, chi never routes such a path into it (404 first) - // no /foo/.../v1.0/drives/...:/... can over-match. name: "colon-syntax under a junk prefix does not match (anchored on HTTP root)", urlPath: "/foo/graph/v1.0/drives/" + testDriveID + "/root:/Documents:/children", @@ -267,6 +267,29 @@ func TestResolveGraphPath(t *testing.T) { expectHit: "item", expectItemID: testItemID, }, + { + // A ':' inside a file name is data, not a delimiter (the delimiter + // is ":/"). This must resolve the whole path including the colon and + // route to the bare item. + name: "colon inside a file name is kept in the path", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/folder1/re:port.txt", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "item", + expectItemID: testItemID, + }, + { + // Colon inside the file name AND a real ":/"-delimited suffix: the + // name keeps its colon, the suffix still routes. + name: "colon in file name with a real suffix", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/folder1/re:port.txt:/children", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "children", + expectItemID: testItemID, + }, { name: "item-anchored colon syntax rewrites", urlPath: "/graph/v1.0/drives/" + testDriveID + "/items/" + testItemID + ":/notes.txt:/children", @@ -394,6 +417,21 @@ func TestResolveGraphPath_DecodesEncodedPath(t *testing.T) { urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents/children", expectedStat: "./Documents/children", }, + { + // A ':' inside a file name (delimiter is ":/") must reach Stat as + // part of the path, not be treated as a path/suffix separator. + name: "colon inside a file name reaches Stat as part of the path", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/folder1/re:port.txt", + expectedStat: "./folder1/re:port.txt", + }, + { + // A ':' at a segment boundary is ambiguous raw, so it must be + // percent-encoded. "%3A" is never seen as the ":/" delimiter and + // decodes back to a literal ':' - here a directory named "weird:". + name: "percent-encoded colon (%3A) at a boundary reaches Stat as a literal colon", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/weird%3A/file.txt", + expectedStat: "./weird:/file.txt", + }, } for _, tt := range tests { diff --git a/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature b/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature index d8e445f769..6a725a6bb6 100644 --- a/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature +++ b/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature @@ -12,6 +12,13 @@ Feature: colon-syntax path lookup on the Graph API collapse to 404 so the middleware never discloses the existence of resources the caller is not allowed to see. + The delimiter is ":/", so a raw ':' inside a file or directory name is kept + as part of the path. A ':' at a segment boundary (e.g. a name ending in ':') + is ambiguous and must be percent-encoded as "%3A" to be addressed. MS Graph + and OneDrive sidestep this by forbidding ':' in names entirely; OpenCloud + allows it (via WebDAV), so the "%3A" escape is how such names are reached + here. + Background: Given user "Alice" has been created with default attributes And user "Alice" has created folder "folder1"