Skip to content

Commit

Permalink
fix: check actual asset file size, not base64 encoded size
Browse files Browse the repository at this point in the history
Previously we were checking whether the base64 encoded size of an asset was too large (>25MiB).
But base64 takes up more space than a normal file, so this was too aggressive.
  • Loading branch information
petebacondarwin committed Jan 24, 2022
1 parent b021eb8 commit 522d1a6
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 13 deletions.
8 changes: 8 additions & 0 deletions .changeset/silly-students-live.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"wrangler": patch
---

fix: check actual asset file size, not base64 encoded size

Previously we were checking whether the base64 encoded size of an asset was too large (>25MiB).
But base64 takes up more space than a normal file, so this was too aggressive.
28 changes: 19 additions & 9 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,17 +373,24 @@ describe("publish", () => {
});

it("should error if the asset is over 25Mb", async () => {
const veryLargeAsset = {
filePath: "large-file.txt",
content: "X".repeat(25 * 1024 * 1024 + 1),
};
const assets = [
{
filePath: "large-file.txt",
// This file is greater than 25MiB when base64 encoded but small enough to be uploaded.
content: "X".repeat(25 * 1024 * 1024 * 0.8 + 1),
},
{
filePath: "too-large-file.txt",
content: "X".repeat(25 * 1024 * 1024 + 1),
},
];
const kvNamespace = {
title: "__test-name_sites_assets",
id: "__test-name_sites_assets-id",
};
writeWranglerToml("./index.js", "./assets", undefined, ["file-1.txt"]);
writeEsmWorkerSource();
writeAssets("./assets", [veryLargeAsset]);
writeAssets("./assets", assets);
mockUploadWorkerRequest();
mockSubDomainRequest();
mockListKVNamespacesRequest(kvNamespace);
Expand All @@ -394,11 +401,14 @@ describe("publish", () => {
expect(stdout).toMatchInlineSnapshot(
`"uploading assets/large-file.txt..."`
);
expect(stderr).toMatchInlineSnapshot(
`"File assets/large-file.txt is too big, it should be under 25 mb."`
);
expect(stderr).toMatchInlineSnapshot(`
"File assets/too-large-file.txt is too big, it should be under 25 MiB. See https://developers.cloudflare.com/workers/platform/limits#kv-limits
%s
If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
`);
expect(error).toMatchInlineSnapshot(
`[Error: File assets/large-file.txt is too big, it should be under 25 mb.]`
`[Error: File assets/too-large-file.txt is too big, it should be under 25 MiB. See https://developers.cloudflare.com/workers/platform/limits#kv-limits]`
);
});
});
Expand Down
17 changes: 13 additions & 4 deletions packages/wrangler/src/sites.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import crypto from "node:crypto";
import { createReadStream } from "node:fs";
import * as path from "node:path";
import { readdir, readFile } from "node:fs/promises";
import { readdir, readFile, stat } from "node:fs/promises";
import ignore from "ignore";
import { fetchResult } from "./cfetch";
import type { Config } from "./config";
Expand Down Expand Up @@ -127,14 +127,14 @@ export async function syncAssets(
if (exclude(relativePath)) {
continue;
}

await validateAssetSize(file);

const { assetKey } = await hashAsset(file);
// now put each of the files into kv
if (!keys.has(assetKey)) {
console.log(`uploading ${file}...`);
const content = await readFile(file, "base64");
if (content.length > 25 * 1024 * 1024) {
throw new Error(`File ${file} is too big, it should be under 25 mb.`);
}
upload.push({
key: assetKey,
value: content,
Expand All @@ -159,6 +159,15 @@ function createPatternMatcher(
}
}

async function validateAssetSize(filePath: string) {
const { size } = await stat(filePath);
if (size > 25 * 1024 * 1024) {
throw new Error(
`File ${filePath} is too big, it should be under 25 MiB. See https://developers.cloudflare.com/workers/platform/limits#kv-limits`
);
}
}

/**
* Information about the assets that should be uploaded
*/
Expand Down

0 comments on commit 522d1a6

Please sign in to comment.