diff --git a/services/graph/pkg/middleware/path_lookup.go b/services/graph/pkg/middleware/path_lookup.go index 695e8b988b..1fc89bcd6c 100644 --- a/services/graph/pkg/middleware/path_lookup.go +++ b/services/graph/pkg/middleware/path_lookup.go @@ -234,26 +234,21 @@ func rewriteColonPath( // /root:/[:/][:] // /items/:/[:/][:] // -// 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: +// The structural delimiter is ":/" (a colon immediately followed by the +// leading slash of the path or suffix); a trailing ":" is the no-suffix +// terminator. 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. +// Clients must percent-encode each path segment, exactly as MS Graph requires +// (an unencoded path is ambiguous). The one OpenCloud-specific point: ':' must +// be encoded as "%3A". The split is on the literal ":/", so an encoded ':' is +// never a delimiter and decodes back to ':' with the rest of the path (e.g. +// "/root:/weird%3A/file.txt" -> "/weird:/file.txt"). OpenCloud allows ':' in +// names whereas MS Graph/OneDrive forbid it, hence this extra character. // // The returned fields are the raw (still percent-encoded) substrings; the // caller decodes them. diff --git a/services/graph/pkg/middleware/path_lookup_test.go b/services/graph/pkg/middleware/path_lookup_test.go index ee36f1d41b..fa866d2fed 100644 --- a/services/graph/pkg/middleware/path_lookup_test.go +++ b/services/graph/pkg/middleware/path_lookup_test.go @@ -267,29 +267,6 @@ 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", @@ -418,17 +395,10 @@ func TestResolveGraphPath_DecodesEncodedPath(t *testing.T) { 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", + // Clients percent-encode ':' as "%3A" to put it in a name. "%3A" is + // never seen as the ":/" delimiter and decodes back to a literal ':' + // - here a directory named "weird:". + name: "percent-encoded colon (%3A) reaches Stat as a literal colon", urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/weird%3A/file.txt", expectedStat: "./weird:/file.txt", }, diff --git a/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature b/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature index 6a725a6bb6..6cd45ab8b1 100644 --- a/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature +++ b/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature @@ -12,12 +12,11 @@ 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. + Clients must percent-encode each path segment, exactly as MS Graph requires. + The only OpenCloud-specific point: ':' must be encoded as "%3A" (the split is + on the literal ":/", so an encoded ':' is never a delimiter). OpenCloud allows + ':' in names whereas MS Graph/OneDrive forbid it, so "%3A" is the one extra + character to encode. Background: Given user "Alice" has been created with default attributes