🐛 BUG: wrangler kv:key put does not work as expected

[vilvei]

Which Cloudflare product(s) does this pertain to?

KV, Wrangler

What version of Wrangler are you using?

2.12.3

What operating system are you using?

Linux

Describe the Bug

wrangler kv:key put does not work as expected when using --path with --metadata commandline options.
Meaning of "as expected": the same bytes can not be read with kv:key get, that were previously put.

Simple way to test:

This works: the same file can be get, which were put:
md5 original.png
81a3478c5f2b12823f3ff0253c3388e7
wrangler kv:key put --binding=FILES kvimage.png --path=original.png
wrangler kv:key get --binding=FILES kvimage.png > copy.png
md5 copy.png
81a3478c5f2b12823f3ff0253c3388e7

This does NOT work, md5 does not match, the bytes are different:
md5 original.png
81a3478c5f2b12823f3ff0253c3388e7
wrangler kv:key put --binding=FILES kvimage.png --path=original.png --metadata='{"contentType":"image/png"}'
wrangler kv:key get --binding=FILES kvimage.png > copy.png
md5 copy.png
40621887293ab018c535120fe0b6bf92

Looking code:

I think the problem is that wrangler is putting the Buffer data into FormData, to be sent to KV. And this is a wrong type of data to be added into FormData.

--path file is read with readFileSyncToBuffer, which returns node-Buffer
https://github.com/cloudflare/workers-sdk/blob/main/packages/wrangler/src/kv/index.ts#L236

This is given to function putKVKeyValue
https://github.com/cloudflare/workers-sdk/blob/main/packages/wrangler/src/kv/helpers.ts#L203

which tries to append the Buffer into undici FormData object.
https://github.com/cloudflare/workers-sdk/blob/main/packages/wrangler/src/kv/helpers.ts#L193

undici FormData append expects value to be eg. Blob or File, but receives Buffer. So Buffer is converted to string.
https://github.com/nodejs/undici/blob/main/types/formdata.d.ts#L27

[vilvei]

The cleanest way to fix this, is to send metadata in PUT-headers: KV can hold only a very small size of metadata-data anyway, so this would not brake anything on storing data.
This also enables the value always to be sent as PUT-body. Which also might help to deliver filedata as ReadStream on PUT-body, instead of potentially 25MB of Buffer in memory.

[scottmas]

Can verify this is happening. Had me utterly stumped for a long while.

[gmemstr]

Linking the culprit with a specific commit hash so it doesn't move around when the file is modified :)

workers-sdk/packages/wrangler/src/kv/helpers.ts

Lines 199 to 207 in 9a46e03

	function asFormData(fields: Record<string, unknown>): FormData {
	const formData = new FormData();
	for (const [name, value] of Object.entries(fields)) {
	formData.append(name, value);
	}
	return formData;
	}

I think we can get away with checking if the type is a Buffer, and if so, convert it? Something like

function asFormData(fields: Record<string, unknown>): FormData {
	const formData = new FormData();

	for (let [name, value] of Object.entries(fields)) {
                if (Buffer.isBuffer(value)) {
                    value = new Blob([value]);
                }
		formData.append(name, value);
	}

	return formData;
}

Maybe there's a more elegant way of doing it, but I'll open a PR with this change to at least get things moving in the right direction.