diff --git a/services/graph/pkg/middleware/path_lookup.go b/services/graph/pkg/middleware/path_lookup.go index 796d19ec8e..43bb25f174 100644 --- a/services/graph/pkg/middleware/path_lookup.go +++ b/services/graph/pkg/middleware/path_lookup.go @@ -248,33 +248,37 @@ func rewriteColonPath( func parseColonPath(routePath string) (colonMatch, bool) { var m colonMatch - // rest is everything after the anchor and its delimiting colon, i.e. - // "/[:/][:]". - var rest string + // The anchor is separated from the rest by the first colon; no colon at all + // means this isn't colon syntax and should pass through. + anchor, rest, found := strings.Cut(routePath, ":") + if !found { + return m, false + } + switch { - case strings.HasPrefix(routePath, "/root:"): - // Root-anchored: the anchor is the {driveID} route param supplied by - // the caller, so there's nothing item-specific to capture here. - rest = strings.TrimPrefix(routePath, "/root:") - case strings.HasPrefix(routePath, "/items/"): - // Item-anchored: the itemID is a single segment terminated by ':'. If a - // '/' appears before any ':', this is an ordinary /items/{id}/... - // request, not colon syntax. - after := strings.TrimPrefix(routePath, "/items/") - itemID, tail, found := strings.Cut(after, ":") - if !found || strings.Contains(itemID, "/") { + 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. + 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. + itemID := strings.TrimPrefix(anchor, "/items/") + if itemID == "" || strings.Contains(itemID, "/") { return m, false } m.isItemAnchored = true m.itemAnchorID = itemID - rest = tail default: return m, false } - // Drop a single optional trailing ':' (the "...:" no-suffix shape), then - // split path from suffix on the one remaining delimiter colon, if any. - // strings.Cut leaves suffix empty when there's no delimiter. + // 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, ":") @@ -284,7 +288,7 @@ func parseColonPath(routePath string) (colonMatch, bool) { if len(m.relPath) < 2 || m.relPath[0] != '/' { return m, false } - if m.suffix != "" && (m.suffix[0] != '/' || strings.IndexByte(m.suffix, ':') >= 0) { + if m.suffix != "" && (m.suffix[0] != '/' || 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 531de52f8c..b933c81a9d 100644 --- a/services/graph/pkg/middleware/path_lookup_test.go +++ b/services/graph/pkg/middleware/path_lookup_test.go @@ -253,6 +253,20 @@ func TestResolveGraphPath(t *testing.T) { expectHit: "children", expectItemID: testItemID, }, + { + // A trailing segment that looks like a suffix ("children") is only a + // suffix when a SECOND colon introduces it. Without it, the whole + // thing is the path, so this must resolve to the bare item (GET "/"), + // NOT to /items/{id}/children. If the parser wrongly split it, the + // request would land on the "children" leaf and this fails. + name: "trailing suffix-looking segment without a second colon stays in the path", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents/children", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "item", + expectItemID: testItemID, + }, { name: "item-anchored colon syntax rewrites", urlPath: "/graph/v1.0/drives/" + testDriveID + "/items/" + testItemID + ":/notes.txt:/children", @@ -372,6 +386,14 @@ func TestResolveGraphPath_DecodesEncodedPath(t *testing.T) { urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/a%252Fb:/children", expectedStat: "./a%2Fb", }, + { + // Without a second colon the whole remainder is the path: Stat must + // receive "/Documents/children", not "/Documents" (which would mean + // "/children" was wrongly treated as a suffix). + name: "suffix-looking segment without a second colon is part of the Stat path", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents/children", + expectedStat: "./Documents/children", + }, } for _, tt := range tests { @@ -385,13 +407,12 @@ func TestResolveGraphPath_DecodesEncodedPath(t *testing.T) { }). Return(statResponse(cs3rpc.Code_CODE_OK, true), nil) - router, cap := newGraphTestRouter(t, gw) + router, _ := newGraphTestRouter(t, gw) req := httptest.NewRequest(http.MethodGet, "http://localhost"+tt.urlPath, nil) rr := httptest.NewRecorder() router.ServeHTTP(rr, req) assert.Equal(t, http.StatusOK, rr.Code) - assert.Equal(t, "children", cap.hit) assert.Equal(t, tt.expectedStat, statPath, "path passed to CS3 Stat must be decoded exactly once") }) }