Skip to content

Commit

Permalink
fix(KeyValueStore): big buffers should not crash (#1734)
Browse files Browse the repository at this point in the history
Closes #1732 
Closes #1710 

Simple fix really, tell shapeshift inputs can be buffers too (I'll also
see if we can add a generic "just check that its an object" in
shapeshift for times like these)
  • Loading branch information
vladfrangu authored Jan 12, 2023
1 parent 432904a commit 2f682f7
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 1 deletion.
19 changes: 19 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"postci:build": "node ./scripts/typescript_fixes.mjs",
"test": "jest --silent",
"test:e2e": "node test/e2e/run.mjs",
"test:full": "cross-env CRAWLEE_DIFFICULT_TESTS=1 jest --silent",
"coverage": "jest --coverage",
"publish:next": "lerna publish from-package --contents dist --dist-tag next --force-publish",
"release:next": "npm run build && npm run publish:next",
Expand Down Expand Up @@ -80,6 +81,7 @@
"basic-auth-parser": "^0.0.2",
"body-parser": "^1.20.0",
"commitlint": "^17.1.2",
"cross-env": "^7.0.3",
"deep-equal": "^2.0.5",
"eslint": "^8.24.0",
"express": "^4.18.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export class KeyValueStoreClient extends BaseClient {
async setRecord(record: storage.KeyValueStoreRecord): Promise<void> {
s.object({
key: s.string.lengthGreaterThan(0),
value: s.union(s.null, s.string, s.number, s.object({}).passthrough),
value: s.union(s.null, s.string, s.number, s.instance(Buffer), s.object({}).passthrough),
contentType: s.string.lengthGreaterThan(0).optional,
}).parse(record);

Expand Down
45 changes: 45 additions & 0 deletions packages/memory-storage/test/no-crash-on-big-buffers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// https://github.com/apify/crawlee/issues/1732
// https://github.com/apify/crawlee/issues/1710

import { rm } from 'node:fs/promises';
import { resolve } from 'node:path';
import { MemoryStorage } from '@crawlee/memory-storage';
import type { KeyValueStoreClient, KeyValueStoreInfo } from '@crawlee/types';

describe('MemoryStorage should not crash when saving a big buffer', () => {
const tmpLocation = resolve(__dirname, './tmp/no-buffer-crash');
const storage = new MemoryStorage({
localDataDirectory: tmpLocation,
persistStorage: false,
});

let kvs: KeyValueStoreInfo;
let store: KeyValueStoreClient;

beforeAll(async () => {
kvs = await storage.keyValueStores().getOrCreate();
store = storage.keyValueStore(kvs.id);
});

afterAll(async () => {
await rm(tmpLocation, { force: true, recursive: true });
});

test('should not crash when saving a big buffer', async () => {
let zip: Buffer;

if (process.env.CRAWLEE_DIFFICULT_TESTS) {
const numbers = Array.from(([...Array(18_100_000).keys()]).map((i) => i * 3_000_000));

zip = Buffer.from([...numbers]);
} else {
zip = Buffer.from([...Array(100_000)].map((i) => i * 8));
}

try {
await store.setRecord({ key: 'owo.zip', value: zip });
} catch (err) {
expect(err).not.toBeDefined();
}
});
});

0 comments on commit 2f682f7

Please sign in to comment.