Skip to content

Commit

Permalink
Fix KVNamespace#put() with empty value
Browse files Browse the repository at this point in the history
Empty `put()`s may send HTTP requests with `null` bodies, violating
an assumption we had. This change handles this case, and adds some
additional tests for empty values.

Closes #703
  • Loading branch information
mrbbot committed Oct 5, 2023
1 parent 7d0ed16 commit 112fa5b
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 4 deletions.
15 changes: 11 additions & 4 deletions packages/miniflare/src/workers/kv/namespace.worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,20 @@ export class KVNamespaceObject extends MiniflareDurableObject {
// through a transform stream to count it (trusting `workerd` to send
// correct value here).
let value = req.body;
assert(value !== null);
// Safety of `!`: `parseInt(null)` is `NaN`
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const contentLength = parseInt(req.headers.get("Content-Length")!);
const valueLengthHint = Number.isNaN(contentLength)
? undefined
: contentLength;
let valueLengthHint: number | undefined;
if (!Number.isNaN(contentLength)) valueLengthHint = contentLength;
else if (value === null) valueLengthHint = 0;

// Empty values may be put with `null` bodies:
// https://github.com/cloudflare/miniflare/issues/703
value ??= new ReadableStream<Uint8Array>({
start(controller) {
controller.close();
},
});

const maxValueSize = this.beingTested
? KVLimits.MAX_VALUE_SIZE_TEST
Expand Down
12 changes: 12 additions & 0 deletions packages/miniflare/test/plugins/cache/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,18 @@ test("match returns cached responses", async (t) => {
t.is(res.status, 200);
t.is(await res.text(), "buffered");
});
test("match returns empty response", async (t) => {
const cache = t.context.caches.default;
const key = "http://localhost/cache-empty";
const resToCache = new Response(null, {
headers: { "Cache-Control": "max-age=3600" },
});
await cache.put(key, resToCache);
const res = await cache.match(key);
assert(res !== undefined);
t.is(res.status, 200);
t.is(await res.text(), "");
});
test("match returns nothing on cache miss", async (t) => {
const cache = t.context.caches.default;
const key = "http://localhost/cache-miss";
Expand Down
7 changes: 7 additions & 0 deletions packages/miniflare/test/plugins/kv/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,13 @@ test("put: puts value", async (t) => {
const results = await kv.list({ prefix: ns });
t.is(results.keys[0]?.expiration, TIME_FUTURE);
});
test("put: puts empty value", async (t) => {
// https://github.com/cloudflare/miniflare/issues/703
const { kv } = t.context;
await kv.put("key", "");
const value = await kv.get("key");
t.is(value, "");
});
test("put: overrides existing keys", async (t) => {
const { kv } = t.context;
await kv.put("key", "value1");
Expand Down
8 changes: 8 additions & 0 deletions packages/miniflare/test/plugins/r2/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,14 @@ test("put: returns metadata for created object", async (t) => {
t.is(object.range, undefined);
isWithin(t, WITHIN_EPSILON, object.uploaded.getTime(), start);
});
test("put: puts empty value", async (t) => {
const { r2 } = t.context;
const object = await r2.put("key", "");
assert(object !== null);
t.is(object.size, 0);
const objectBody = await r2.get("key");
t.is(await objectBody?.text(), "");
});
test("put: overrides existing keys", async (t) => {
const { r2, ns, object } = t.context;
await r2.put("key", "value1");
Expand Down

0 comments on commit 112fa5b

Please sign in to comment.