Skip to content

Commit 638b970

Browse files
committed
src: cache compiled node.config.json schema and align with runtime
1 parent 7aa4b1b commit 638b970

File tree

4 files changed

+39
-27
lines changed

4 files changed

+39
-27
lines changed

src/node_config_file.cc

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -244,30 +244,25 @@ ParseResult ConfigReader::ParseConfig(const std::string_view& config_path) {
244244
return ParseResult::InvalidContent;
245245
}
246246

247-
// Validate against JSON Schema after basic parsing succeeds.
248-
// This catches type errors in properties before the option
249-
// parsing loop, which would otherwise produce less clear messages.
247+
// Validate against JSON Schema after basic parsing succeeds. This catches
248+
// unknown namespaces and type errors in properties before the option parsing
249+
// loop, which would otherwise produce less clear messages (or silently skip
250+
// unknown top-level keys). kNodeConfigSchema is a compile-time constant, so
251+
// compile it once on first call.
250252
{
251-
auto schema = ata::compile(kNodeConfigSchema);
252-
if (schema) {
253-
auto result = ata::validate(schema, file_content);
254-
if (!result.valid) {
255-
for (const auto& err : result.errors) {
256-
if (err.code != ata::error_code::additional_property_not_allowed) {
257-
FPrintF(
258-
stderr, "Invalid configuration in %s:\n", config_path.data());
259-
for (const auto& e : result.errors) {
260-
if (e.code != ata::error_code::additional_property_not_allowed) {
261-
FPrintF(stderr,
262-
" %s: %s\n",
263-
e.path.empty() ? "/" : e.path,
264-
e.message);
265-
}
266-
}
267-
return ParseResult::InvalidContent;
268-
}
269-
}
253+
static const ata::schema_ref compiled_schema =
254+
ata::compile(kNodeConfigSchema);
255+
CHECK(compiled_schema);
256+
auto result = ata::validate(compiled_schema, file_content);
257+
if (!result.valid) {
258+
FPrintF(stderr, "Invalid configuration in %s:\n", config_path.data());
259+
for (const auto& err : result.errors) {
260+
FPrintF(stderr,
261+
" %s: %s\n",
262+
err.path.empty() ? "/" : err.path,
263+
err.message);
270264
}
265+
return ParseResult::InvalidContent;
271266
}
272267
}
273268

src/node_config_schema.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,12 @@
44
static constexpr const char kNodeConfigSchema[] = R"JSON(
55
{
66
"$schema": "https://json-schema.org/draft/2020-12/schema",
7-
"additionalProperties": false,
87
"required": [],
98
"properties": {
109
"$schema": {
1110
"type": "string"
1211
},
1312
"nodeOptions": {
14-
"additionalProperties": false,
1513
"required": [],
1614
"properties": {
1715
"addons": {
@@ -761,7 +759,6 @@ static constexpr const char kNodeConfigSchema[] = R"JSON(
761759
},
762760
"permission": {
763761
"type": "object",
764-
"additionalProperties": false,
765762
"required": [],
766763
"properties": {
767764
"allow-addons": {
@@ -826,7 +823,6 @@ static constexpr const char kNodeConfigSchema[] = R"JSON(
826823
},
827824
"test": {
828825
"type": "object",
829-
"additionalProperties": false,
830826
"required": [],
831827
"properties": {
832828
"experimental-test-coverage": {
@@ -991,7 +987,6 @@ static constexpr const char kNodeConfigSchema[] = R"JSON(
991987
},
992988
"watch": {
993989
"type": "object",
994-
"additionalProperties": false,
995990
"required": [],
996991
"properties": {
997992
"watch": {
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"nodeOptions": {
3+
"addons": "not-a-boolean",
4+
"max-http-header-size": "not-a-number"
5+
}
6+
}

test/parallel/test-config-file.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,7 @@ describe('JSON Schema validation', () => {
614614
'-p', '"Hello"',
615615
]);
616616
assert.match(result.stderr, /Invalid configuration/);
617+
assert.match(result.stderr, /\/nodeOptions\/addons/);
617618
assert.strictEqual(result.code, 9);
618619
});
619620

@@ -624,6 +625,7 @@ describe('JSON Schema validation', () => {
624625
'-p', '"Hello"',
625626
]);
626627
assert.match(result.stderr, /Invalid configuration/);
628+
assert.match(result.stderr, /\/nodeOptions\/max-http-header-size/);
627629
assert.strictEqual(result.code, 9);
628630
});
629631

@@ -634,6 +636,7 @@ describe('JSON Schema validation', () => {
634636
'-p', '"Hello"',
635637
]);
636638
assert.match(result.stderr, /Invalid configuration/);
639+
assert.match(result.stderr, /\/nodeOptions\/import/);
637640
assert.strictEqual(result.code, 9);
638641
});
639642

@@ -644,6 +647,19 @@ describe('JSON Schema validation', () => {
644647
'-p', '"Hello"',
645648
]);
646649
assert.match(result.stderr, /Invalid configuration/);
650+
assert.match(result.stderr, /\/nodeOptions\/import/);
651+
assert.strictEqual(result.code, 9);
652+
});
653+
654+
test('reports every error when multiple properties fail', async () => {
655+
const result = await spawnPromisified(process.execPath, [
656+
'--experimental-config-file',
657+
fixtures.path('rc/invalid-schema-multiple-errors.json'),
658+
'-p', '"Hello"',
659+
]);
660+
assert.match(result.stderr, /Invalid configuration/);
661+
assert.match(result.stderr, /\/nodeOptions\/addons/);
662+
assert.match(result.stderr, /\/nodeOptions\/max-http-header-size/);
647663
assert.strictEqual(result.code, 9);
648664
});
649665

0 commit comments

Comments
 (0)