KV keys with / with kvPersist throws EISDIR

[mekanoe]

When using kvPersist, it writes to disk by filename matching 1:1 with the entry key, keys with / included will write out a directory structure.

This means creating key like foo/bar creates a file .mf/kv/TEST_NAMESPACE/foo/bar, then also creating a key like foo will find .mf/kv/TEST_NAMESPACE/foo to already be a directory, and Node.js throws EISDIR: illegal operation on a directory.

This doesn't apply to a leading / as miniflare already translates it to _, e.g. in /hello-world. Other sanitization steps seems to do it as well, e.g. foo:bar and foo will do this as well.

Workers KV itself doesn't have this issue.

Minimal reproduction: https://github.com/kayteh/miniflare-kv-slash

In a real world example, one might use a few different keys to relate to each other loosely:

• users/bob
• users/bob/messages

[mrbbot]

Hey! 👋 Which version of Miniflare are you using? This is a limitation of Miniflare's file storage format, but this case should be detected and the error message thrown should explain this:

miniflare/packages/storage-file/src/index.ts

Lines 107 to 115 in ab60cad

	throw new FileStorageError(
	"ERR_NAMESPACE_KEY_CHILD",
	'Cannot put key "' +
	key +
	'" as a parent namespace is also a key.\n' +
	"This is a limitation of Miniflare's file-system storage. Please " +
	"use in-memory/Redis storage instead, or change your key layout.",
	e
	);

[BtheGit]

The problem with this seems to be that Workers KV uses colon-separated prefixes in its examples. That may be a source of problems for people (like myself) who just took it as expected convention and then subsequently couldn't use the local pages dev environment because of the aforementioned issue.

[SupremeTechnopriest]

Hey @mrbbot Im seeing something similar with custom cache keys. If the cache key contains a URL, it looks like it gets parsed into a directory structure.

http://www.test.com/:anotherKey

results in

cache/default/http/www/test/com/anotherkey

Then when trying to read from cache the following error is thrown:

[Error: EISDIR: illegal operation on a directory, read] {
  errno: -21,
  code: 'EISDIR',
  syscall: 'read'
}

If I set cache key to a string without a url and without a : separator it doesnt throw the error.

[SupremeTechnopriest]

Maybe we could replace : and / with an _ instead?

[aboodman]

@mrbbot - this also happens with the transactional storage API in miniflare. We just spent 2h debugging something that emits this same error when slashes are used as keys in the storage api. FYI :).

[phritz]

FYI :)

I don't think that's what we mean. I think we mean:

Please fix this. We just spent hours debugging this with one of our customers who uses slashes in their keys. Turns out this bug has been known to CF for the better part of a year. It's a totally natural convention to use slashes in keys eg "users/foo" and the APIs accept it as a key even though it can cause these kinds of collisions in the filesystem (and thus be impossible to read back out).

[SupremeTechnopriest]

Any progress on this one?

[SupremeTechnopriest]

Here is my current workaround that I use for Miniflare:

cacheKey.replaceAll('/', '_').replaceAll(':', '_')

Wondering if this is a behavior mismatch. Does the worker runtime not allow for / and : as well? URLs as cache keys seem to work ok in production.

[mekanoe]

Couldn't the miniflare KV storage just base64 or url encode keys? It's storing them as 1:1 paths right now. You write a key like KV.put("hello/world", ...) and it outputs to hello/world in persisted dir. This would mean it's making a further directory in the tree, causing KV.put("hello") to end up being a directory, something miniflare isn't expecting.

URL encoding would allow for key scanning, just translate any / or \ as %2F or %5C respectively. All path-safe, as far as I know. Technically more path safe than before too, since you can make a key a number of arbitrary symbols, something POSIX and Windows don't really enjoy doing

[mrbbot]

Hey! 👋 I been thinking about switching out the storage system in Miniflare 3. I've put some thoughts together here: #525.

[mrbbot]

Hey! 👋 This should be fixed in Miniflare 3, which uses the same workerd runtime as production, so shouldn't have these behaviour mismatches. I'm going to close this as it's unlikely we'll have time to fix this in Miniflare 2. Our development effort is primarily focused on version 3.