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 (>25Mb).
But base64 takes up more space than a normal file, so this was too aggressive.
  • Loading branch information
petebacondarwin committed Jan 20, 2022
1 parent 3ed29da commit 88b57c4
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 11 deletions.
7 changes: 7 additions & 0 deletions .changeset/silly-students-live.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"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 (>25Mb). But base64 takes up more space than a normal file, so this was too aggressive.
21 changes: 14 additions & 7 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 25Mb 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 @@ -395,10 +402,10 @@ describe("publish", () => {
`"uploading assets/large-file.txt..."`
);
expect(stderr).toMatchInlineSnapshot(
`"File assets/large-file.txt is too big, it should be under 25 mb."`
`"File assets/too-large-file.txt is too big, it should be under 25 mb."`
);
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 mb.]`
);
});
});
Expand Down
15 changes: 11 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 { AssetPaths } 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 @@ -158,3 +158,10 @@ function createPatternMatcher(
return (filePath) => ignorer.test(filePath).ignored;
}
}

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 mb.`);
}
}

0 comments on commit 88b57c4

Please sign in to comment.