refactor(graph): clearer colon-path anchor parsing, pin second-colon rule

parseColonPath split the anchor asymmetrically (root trimmed the delimiter
colon as part of a literal "/root:" prefix, item cut on it), and the root
branch's comment talked about the driveID which isn't this function's
concern. Split once at the first colon into anchor + rest, then classify
the anchor (exactly "/root", or "/items/{id}" with a single-segment id).
Same behavior, easier to follow.

Also add explicit coverage for the rule that a suffix requires a second
colon: "/root:/Documents/children" (no second colon) is the path
"/Documents/children", not the path "/Documents" with a "/children"
suffix. Two tests pin it end-to-end - it must route to the bare item (not
/items/{id}/children) and Stat must receive the full path.
This commit is contained in:
Dominik Schmidt
2026-07-01 11:54:03 +02:00
committed by Ralf Haferkamp
parent 4b98022688
commit 407bb2bc70
2 changed files with 46 additions and 21 deletions

View File

@@ -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.
// "/<path>[:/<suffix>][:]".
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 "/<path>[:/<suffix>][:]". 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

View File

@@ -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")
})
}