owncloud / ocis

:atom_symbol: ownCloud Infinite Scale Stack
https://doc.owncloud.com/ocis/next/
Apache License 2.0
1.36k stars 179 forks source link

nats: invalid key when trying to read or write a file #9114

Closed butonic closed 4 months ago

butonic commented 4 months ago

running ocis master and reva edge I get this error line:

2024-05-08T17:12:59+02:00 ERR error reading blobsize xattr, using 0 error="error reading blobsize xattr: Failed to store data in bucket 'spaces/so/me-admin-user-id-0000-000000000000/nodes/04/a5/b4/86/-4ce3-492e-a98e-87eddada0146.REV.2024-05-08T14:54:24.01526906Z': nats: invalid key" name=-4ce3-492e-a98e-87eddada0146.REV.2024-05-08T14:54:24.01526906Z pkg=rgrpc service=storage-users traceid=a8a634abcc56ef9bb072699c85e5de3e

and indeed, : is not allowed as a key: https://github.com/nats-io/nats.go/blame/main/kv.go#L348

validKeyRe       = regexp.MustCompile(`^[-/_=\.a-zA-Z0-9]+$`)

yes the regex was touched a few weeks ago, but the previous one also did not allow :: https://github.com/nats-io/nats.go/blame/9d4b227179d60d6996c32b4b889d4e325ee06f78/kv.go#L348

validKeyRe    = regexp.MustCompile(`\A[-/_=\.a-zA-Z0-9]+\z`)

but ... how did this ever work???

the key is spaces/so/me-admin-user-id-0000-000000000000/nodes/04/a5/b4/86/-4ce3-492e-a98e-87eddada0146.REV.2024-05-08T14:54:24.01526906Z

and yeah, this diff fixes it:

diff --git a/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go b/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go
index 5c19821b4..a07f8125e 100644
--- a/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go
+++ b/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go
@@ -324,5 +324,5 @@ func (b MessagePackBackend) cacheKey(path string) string {
        // rootPath is guaranteed to have no trailing slash
        // the cache key shouldn't begin with a slash as some stores drop it which can cause
        // confusion
-       return strings.TrimPrefix(path, b.rootPath+"/")
+       return strings.ReplaceAll(strings.TrimPrefix(path, b.rootPath+"/"), ":", "-")
 }
butonic commented 4 months ago

cc @aduffeck any idea?

kobergj commented 4 months ago

Nats kv store has an option to base32 encode the keys. It just needs to be activated.

butonic commented 4 months ago

STORAGE_USERS_FILEMETADATA_CACHE_STOREdefaults to memonry. I set it to nats-js-kv ... which then uses nats ... which seems broken somehow?

butonic commented 4 months ago

uploading a file in the web ui works, but when editing a textfile in the web editor and trying to save the file it fails.

butonic commented 4 months ago

@aduffeck what can go wrong when STORAGE_USERS_FILEMETADATA_CACHE_STORE is not shared between multiple storageproviders? It is not used in the helm charts ... but IIRC it should be shared because it caches file metadata, right ... and that should use the same cache ... STORAGE_USERS_FILEMETADATA_CACHE_STORE defaults to memory ... which worries me ...

kobergj commented 4 months ago

Issue doesn't impact 5.0.3 but only master. Only reva fix and bump needed.