Skip to content

Commit 027c531

Browse files
MoLowtargos
authored andcommittedMay 30, 2023
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 33aa373 commit 027c531

File tree

3 files changed

+147
-14
lines changed

3 files changed

+147
-14
lines changed
 

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

+8-2
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

+36-12
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,8 @@ function getRunArgs({ path, inspectPort, testNamePatterns }) {
163163
const serializer = new DefaultSerializer();
164164
serializer.writeHeader();
165165
const v8Header = serializer.releaseBuffer();
166-
const kSerializedSizeHeader = 4;
166+
const kV8HeaderLength = TypedArrayPrototypeGetLength(v8Header);
167+
const kSerializedSizeHeader = 4 + kV8HeaderLength;
167168

168169
class FileTest extends Test {
169170
// This class maintains two buffers:
@@ -236,22 +237,42 @@ class FileTest extends Test {
236237
this.#handleReportItem(item);
237238
}
238239
reportStarted() {}
239-
report() {
240+
drain() {
240241
this.#drainRawBuffer();
241242
this.#drainReportBuffer();
243+
}
244+
report() {
245+
this.drain();
242246
const skipReporting = this.#skipReporting();
243247
if (!skipReporting) {
244248
super.reportStarted();
245249
super.report();
246250
}
247251
}
248252
parseMessage(readData) {
249-
const dataLength = TypedArrayPrototypeGetLength(readData);
253+
let dataLength = TypedArrayPrototypeGetLength(readData);
250254
if (dataLength === 0) return;
255+
const partialV8Header = readData[dataLength - 1] === v8Header[0];
256+
257+
if (partialV8Header) {
258+
// This will break if v8Header length (2 bytes) is changed.
259+
// However it is covered by tests.
260+
readData = TypedArrayPrototypeSubarray(readData, 0, dataLength - 1);
261+
dataLength--;
262+
}
251263

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

267-
while (bufferHead && headerIndex !== kSerializedSizeHeader) {
288+
while (bufferHead && headerIndex !== 0) {
268289
const nonSerializedData = headerIndex === -1 ?
269290
bufferHead :
270-
bufferHead.slice(0, headerIndex - kSerializedSizeHeader);
291+
bufferHead.slice(0, headerIndex);
271292
nonSerialized = Buffer.concat([nonSerialized, nonSerializedData]);
272293
this.#rawBufferSize -= TypedArrayPrototypeGetLength(nonSerializedData);
273294
if (headerIndex === -1) {
274295
ArrayPrototypeShift(this.#rawBuffer);
275296
} else {
276-
this.#rawBuffer[0] = bufferHead.subarray(headerIndex - kSerializedSizeHeader);
297+
this.#rawBuffer[0] = TypedArrayPrototypeSubarray(bufferHead, headerIndex);
277298
}
278299
bufferHead = this.#rawBuffer[0];
279300
headerIndex = bufferHead?.indexOf(v8Header);
@@ -295,10 +316,10 @@ class FileTest extends Test {
295316
// We call `readUInt32BE` manually here, because this is faster than first converting
296317
// it to a buffer and using `readUInt32BE` on that.
297318
const fullMessageSize = (
298-
bufferHead[0] << 24 |
299-
bufferHead[1] << 16 |
300-
bufferHead[2] << 8 |
301-
bufferHead[3]
319+
bufferHead[kV8HeaderLength] << 24 |
320+
bufferHead[kV8HeaderLength + 1] << 16 |
321+
bufferHead[kV8HeaderLength + 2] << 8 |
322+
bufferHead[kV8HeaderLength + 3]
302323
) + kSerializedSizeHeader;
303324

304325
if (this.#rawBufferSize < fullMessageSize) break;
@@ -474,4 +495,7 @@ function run(options) {
474495
return root.reporter;
475496
}
476497

477-
module.exports = { run };
498+
module.exports = {
499+
FileTest, // Exported for tests only
500+
run,
501+
};
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)
Please sign in to comment.