Skip to content

Commit 47602fe

Browse files
committed
test_runner: fix test deserialize edge cases
PR-URL: #48106 Fixes: #48103 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent 999a289 commit 47602fe

File tree

3 files changed

+147
-14
lines changed

3 files changed

+147
-14
lines changed

lib/internal/test_runner/reporter/v8-serializer.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
'use strict';
22

3+
const {
4+
TypedArrayPrototypeGetLength,
5+
} = primordials;
36
const { DefaultSerializer } = require('v8');
47
const { Buffer } = require('buffer');
58
const { serializeError } = require('internal/error_serdes');
69

710

811
module.exports = async function* v8Reporter(source) {
912
const serializer = new DefaultSerializer();
13+
serializer.writeHeader();
14+
const headerLength = TypedArrayPrototypeGetLength(serializer.releaseBuffer());
1015

1116
for await (const item of source) {
1217
const originalError = item.data.details?.error;
@@ -16,6 +21,7 @@ module.exports = async function* v8Reporter(source) {
1621
// Error is restored after serialization.
1722
item.data.details.error = serializeError(originalError);
1823
}
24+
serializer.writeHeader();
1925
// Add 4 bytes, to later populate with message length
2026
serializer.writeRawBytes(Buffer.allocUnsafe(4));
2127
serializer.writeHeader();
@@ -26,14 +32,14 @@ module.exports = async function* v8Reporter(source) {
2632
}
2733

2834
const serializedMessage = serializer.releaseBuffer();
29-
const serializedMessageLength = serializedMessage.length - 4;
35+
const serializedMessageLength = serializedMessage.length - (4 + headerLength);
3036

3137
serializedMessage.set([
3238
serializedMessageLength >> 24 & 0xFF,
3339
serializedMessageLength >> 16 & 0xFF,
3440
serializedMessageLength >> 8 & 0xFF,
3541
serializedMessageLength & 0xFF,
36-
], 0);
42+
], headerLength);
3743
yield serializedMessage;
3844
}
3945
};

lib/internal/test_runner/runner.js

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,8 @@ function getRunArgs({ path, inspectPort, testNamePatterns }) {
162162
const serializer = new DefaultSerializer();
163163
serializer.writeHeader();
164164
const v8Header = serializer.releaseBuffer();
165-
const kSerializedSizeHeader = 4;
165+
const kV8HeaderLength = TypedArrayPrototypeGetLength(v8Header);
166+
const kSerializedSizeHeader = 4 + kV8HeaderLength;
166167

167168
class FileTest extends Test {
168169
// This class maintains two buffers:
@@ -235,22 +236,42 @@ class FileTest extends Test {
235236
this.#handleReportItem(item);
236237
}
237238
reportStarted() {}
238-
report() {
239+
drain() {
239240
this.#drainRawBuffer();
240241
this.#drainReportBuffer();
242+
}
243+
report() {
244+
this.drain();
241245
const skipReporting = this.#skipReporting();
242246
if (!skipReporting) {
243247
super.reportStarted();
244248
super.report();
245249
}
246250
}
247251
parseMessage(readData) {
248-
const dataLength = TypedArrayPrototypeGetLength(readData);
252+
let dataLength = TypedArrayPrototypeGetLength(readData);
249253
if (dataLength === 0) return;
254+
const partialV8Header = readData[dataLength - 1] === v8Header[0];
255+
256+
if (partialV8Header) {
257+
// This will break if v8Header length (2 bytes) is changed.
258+
// However it is covered by tests.
259+
readData = TypedArrayPrototypeSubarray(readData, 0, dataLength - 1);
260+
dataLength--;
261+
}
250262

251-
ArrayPrototypePush(this.#rawBuffer, readData);
263+
if (this.#rawBuffer[0] && TypedArrayPrototypeGetLength(this.#rawBuffer[0]) < kSerializedSizeHeader) {
264+
this.#rawBuffer[0] = Buffer.concat([this.#rawBuffer[0], readData]);
265+
} else {
266+
ArrayPrototypePush(this.#rawBuffer, readData);
267+
}
252268
this.#rawBufferSize += dataLength;
253269
this.#proccessRawBuffer();
270+
271+
if (partialV8Header) {
272+
ArrayPrototypePush(this.#rawBuffer, TypedArrayPrototypeSubarray(v8Header, 0, 1));
273+
this.#rawBufferSize++;
274+
}
254275
}
255276
#drainRawBuffer() {
256277
while (this.#rawBuffer.length > 0) {
@@ -263,16 +284,16 @@ class FileTest extends Test {
263284
let headerIndex = bufferHead.indexOf(v8Header);
264285
let nonSerialized = Buffer.alloc(0);
265286

266-
while (bufferHead && headerIndex !== kSerializedSizeHeader) {
287+
while (bufferHead && headerIndex !== 0) {
267288
const nonSerializedData = headerIndex === -1 ?
268289
bufferHead :
269-
bufferHead.slice(0, headerIndex - kSerializedSizeHeader);
290+
bufferHead.slice(0, headerIndex);
270291
nonSerialized = Buffer.concat([nonSerialized, nonSerializedData]);
271292
this.#rawBufferSize -= TypedArrayPrototypeGetLength(nonSerializedData);
272293
if (headerIndex === -1) {
273294
ArrayPrototypeShift(this.#rawBuffer);
274295
} else {
275-
this.#rawBuffer[0] = bufferHead.subarray(headerIndex - kSerializedSizeHeader);
296+
this.#rawBuffer[0] = TypedArrayPrototypeSubarray(bufferHead, headerIndex);
276297
}
277298
bufferHead = this.#rawBuffer[0];
278299
headerIndex = bufferHead?.indexOf(v8Header);
@@ -294,10 +315,10 @@ class FileTest extends Test {
294315
// We call `readUInt32BE` manually here, because this is faster than first converting
295316
// it to a buffer and using `readUInt32BE` on that.
296317
const fullMessageSize = (
297-
bufferHead[0] << 24 |
298-
bufferHead[1] << 16 |
299-
bufferHead[2] << 8 |
300-
bufferHead[3]
318+
bufferHead[kV8HeaderLength] << 24 |
319+
bufferHead[kV8HeaderLength + 1] << 16 |
320+
bufferHead[kV8HeaderLength + 2] << 8 |
321+
bufferHead[kV8HeaderLength + 3]
301322
) + kSerializedSizeHeader;
302323

303324
if (this.#rawBufferSize < fullMessageSize) break;
@@ -473,4 +494,7 @@ function run(options) {
473494
return root.reporter;
474495
}
475496

476-
module.exports = { run };
497+
module.exports = {
498+
FileTest, // Exported for tests only
499+
run,
500+
};
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// Flags: --expose-internals --no-warnings
2+
3+
import '../common/index.mjs';
4+
import { describe, it, beforeEach } from 'node:test';
5+
import assert from 'node:assert';
6+
import { finished } from 'node:stream/promises';
7+
import { DefaultSerializer } from 'node:v8';
8+
import serializer from 'internal/test_runner/reporter/v8-serializer';
9+
import runner from 'internal/test_runner/runner';
10+
11+
async function toArray(chunks) {
12+
const arr = [];
13+
for await (const i of chunks) arr.push(i);
14+
return arr;
15+
}
16+
17+
const chunks = await toArray(serializer([
18+
{ type: 'test:diagnostic', data: { nesting: 0, details: {}, message: 'diagnostic' } },
19+
]));
20+
const defaultSerializer = new DefaultSerializer();
21+
defaultSerializer.writeHeader();
22+
const headerLength = defaultSerializer.releaseBuffer().length;
23+
24+
describe('v8 deserializer', () => {
25+
let fileTest;
26+
let reported;
27+
beforeEach(() => {
28+
reported = [];
29+
fileTest = new runner.FileTest({ name: 'filetest' });
30+
fileTest.reporter.on('data', (data) => reported.push(data));
31+
assert(fileTest.isClearToSend());
32+
});
33+
34+
async function collectReported(chunks) {
35+
chunks.forEach((chunk) => fileTest.parseMessage(chunk));
36+
fileTest.drain();
37+
fileTest.reporter.end();
38+
await finished(fileTest.reporter);
39+
return reported;
40+
}
41+
42+
it('should do nothing when no chunks', async () => {
43+
const reported = await collectReported([]);
44+
assert.deepStrictEqual(reported, []);
45+
});
46+
47+
it('should deserialize a chunk with no serialization', async () => {
48+
const reported = await collectReported([Buffer.from('unknown')]);
49+
assert.deepStrictEqual(reported, [
50+
{ data: { __proto__: null, file: 'filetest', message: 'unknown' }, type: 'test:stdout' },
51+
]);
52+
});
53+
54+
it('should deserialize a serialized chunk', async () => {
55+
const reported = await collectReported(chunks);
56+
assert.deepStrictEqual(reported, [
57+
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
58+
]);
59+
});
60+
61+
it('should deserialize a serialized chunk after non-serialized chunk', async () => {
62+
const reported = await collectReported([Buffer.concat([Buffer.from('unknown'), ...chunks])]);
63+
assert.deepStrictEqual(reported, [
64+
{ data: { __proto__: null, file: 'filetest', message: 'unknown' }, type: 'test:stdout' },
65+
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
66+
]);
67+
});
68+
69+
it('should deserialize a serialized chunk before non-serialized output', async () => {
70+
const reported = await collectReported([Buffer.concat([ ...chunks, Buffer.from('unknown')])]);
71+
assert.deepStrictEqual(reported, [
72+
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
73+
{ data: { __proto__: null, file: 'filetest', message: 'unknown' }, type: 'test:stdout' },
74+
]);
75+
});
76+
77+
const headerPosition = headerLength * 2 + 4;
78+
for (let i = 0; i < headerPosition + 5; i++) {
79+
const message = `should deserialize a serialized message split into two chunks {...${i},${i + 1}...}`;
80+
it(message, async () => {
81+
const data = chunks[0];
82+
const reported = await collectReported([data.subarray(0, i), data.subarray(i)]);
83+
assert.deepStrictEqual(reported, [
84+
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
85+
]);
86+
});
87+
88+
it(`${message} wrapped by non-serialized data`, async () => {
89+
const data = chunks[0];
90+
const reported = await collectReported([
91+
Buffer.concat([Buffer.from('unknown'), data.subarray(0, i)]),
92+
Buffer.concat([data.subarray(i), Buffer.from('unknown')]),
93+
]);
94+
assert.deepStrictEqual(reported, [
95+
{ data: { __proto__: null, file: 'filetest', message: 'unknown' }, type: 'test:stdout' },
96+
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
97+
{ data: { __proto__: null, file: 'filetest', message: 'unknown' }, type: 'test:stdout' },
98+
]);
99+
}
100+
);
101+
}
102+
103+
});

0 commit comments

Comments
 (0)