Skip to content

Commit 961280c

Browse files
jviottijviotti
authored
Only allow extends at the top level of one.json (#851)
This was a mistake on our end. `extends` was supposed to be only at the top level. The docs had it right. Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent a46161b commit 961280c

23 files changed

Lines changed: 172 additions & 146 deletions

‎collections/self/v1/schemas/configuration/page.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@
1616
"description": {
1717
"type": "string"
1818
},
19-
"extends": {
20-
"$ref": "./extends.json"
21-
},
2219
"contents": {
2320
"$ref": "./contents.json"
2421
},

‎collections/self/v1/tests/configuration/configuration.test.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,21 @@
8181
}
8282
}
8383
},
84+
{
85+
"description": "No extends on a page",
86+
"valid": false,
87+
"data": {
88+
"url": "http://localhost:8000",
89+
"contents": {
90+
"example": {
91+
"title": "A sample schema folder",
92+
"description": "For testing purposes",
93+
"github": "sourcemeta/one",
94+
"extends": [ "./foo" ]
95+
}
96+
}
97+
}
98+
},
8499
{
85100
"description": "No includes on a page",
86101
"valid": false,

‎src/configuration/read.cc‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,15 @@ auto dereference(const std::filesystem::path &base,
4343
sourcemeta::core::JSON &input,
4444
const sourcemeta::core::Pointer &location,
4545
std::unordered_set<std::string> &visited,
46-
std::unordered_set<std::string> &all_files) -> void {
46+
std::unordered_set<std::string> &all_files, const bool is_root)
47+
-> void {
4748
assert(base.is_absolute());
4849
if (!input.is_object()) {
4950
return;
5051

51-
// Read extensions
52-
} else if (input.defines("extends") && input.at("extends").is_array()) {
52+
// Read extensions (only at the top level, not inside contents)
53+
} else if (is_root && input.defines("extends") &&
54+
input.at("extends").is_array()) {
5355
auto accumulator{sourcemeta::core::JSON::make_object()};
5456
for (const auto &entry : input.at("extends").as_array()) {
5557
if (entry.is_string()) {
@@ -63,7 +65,8 @@ auto dereference(const std::filesystem::path &base,
6365
all_files.emplace(target_path.native());
6466
auto extension{read_file(base, new_location, target_path)};
6567
if (extension.is_object()) {
66-
dereference(target_path, extension, new_location, visited, all_files);
68+
dereference(target_path, extension, new_location, visited, all_files,
69+
true);
6770
accumulator.merge(std::move(extension).as_object());
6871
}
6972

@@ -75,7 +78,7 @@ auto dereference(const std::filesystem::path &base,
7578
accumulator.merge(input.as_object());
7679
input = std::move(accumulator);
7780
assert(!input.defines("extends"));
78-
dereference(base, input, location, visited, all_files);
81+
dereference(base, input, location, visited, all_files, is_root);
7982

8083
// Read included files
8184
} else if (!location.empty() && input.defines("include") &&
@@ -92,7 +95,7 @@ auto dereference(const std::filesystem::path &base,
9295
}
9396
all_files.emplace(target_path.native());
9497
input.into(read_file(base, new_location, target_path));
95-
dereference(target_path, input, new_location, visited, all_files);
98+
dereference(target_path, input, new_location, visited, all_files, is_root);
9699
visited.erase(target_path.native());
97100

98101
// Revisit and relativize paths
@@ -117,7 +120,8 @@ auto dereference(const std::filesystem::path &base,
117120
[](const auto &entry) { return entry.first; });
118121
for (const auto &key : keys) {
119122
dereference(base, input.at("contents").at(key),
120-
location.concat({"contents", key}), visited, all_files);
123+
location.concat({"contents", key}), visited, all_files,
124+
false);
121125
}
122126
}
123127
}
@@ -198,7 +202,7 @@ auto Configuration::read(const std::filesystem::path &configuration_path,
198202
configuration_files.emplace(canonical_config);
199203
std::unordered_set<std::string> visited;
200204
visited.emplace(canonical_config);
201-
dereference(configuration_path, data, {}, visited, configuration_files);
205+
dereference(configuration_path, data, {}, visited, configuration_files, true);
202206

203207
if (data.is_object() && data.defines("url") && data.defines("contents") &&
204208
data.at("contents").is_object()) {

‎test/cli/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ if(ONE_INDEX)
3030
sourcemeta_one_test_cli(common index fail-invalid-config-json)
3131
sourcemeta_one_test_cli(common index fail-api-false-html-default)
3232
sourcemeta_one_test_cli(common index fail-api-false-html-enabled)
33+
sourcemeta_one_test_cli(common index fail-page-level-extends)
3334
sourcemeta_one_test_cli(common index fail-invalid-configuration)
3435
sourcemeta_one_test_cli(common index fail-invalid-schema)
3536
sourcemeta_one_test_cli(common index fail-invalid-schema-json)

‎test/cli/index/common/fail-page-level-extends.sh‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
#!/bin/sh
2+
3+
set -o errexit
4+
set -o nounset
5+
6+
TMP="$(mktemp -d)"
7+
clean() { rm -rf "$TMP"; }
8+
trap clean EXIT
9+
10+
cat << 'EOF' > "$TMP/one.json"
11+
{
12+
"url": "http://localhost:8000",
13+
"contents": {
14+
"example": {
15+
"title": "Test",
16+
"extends": [ "./other.json" ]
17+
}
18+
}
19+
}
20+
EOF
21+
22+
"$1" --skip-banner "$TMP/one.json" "$TMP/output" 2> "$TMP/output.txt" && CODE="$?" || CODE="$?"
23+
test "$CODE" = "1" || exit 1
24+
25+
cat << EOF > "$TMP/expected.txt"
26+
Writing output to: $(realpath "$TMP")/output
27+
Using configuration: $(realpath "$TMP")/one.json
28+
error: Invalid configuration
29+
at path $(realpath "$TMP")/one.json
30+
The value was expected to be an object that defines the property "include"
31+
at instance location "/contents/example"
32+
at evaluate path "/properties/contents/\$ref/additionalProperties/anyOf/0/required"
33+
The object value was not expected to define the property "extends"
34+
at instance location "/contents/example/extends"
35+
at evaluate path "/properties/contents/\$ref/additionalProperties/anyOf/1/\$ref/additionalProperties"
36+
The object value was not expected to define additional properties
37+
at instance location "/contents/example"
38+
at evaluate path "/properties/contents/\$ref/additionalProperties/anyOf/1/\$ref/additionalProperties"
39+
The object value was expected to validate against the referenced schema
40+
at instance location "/contents/example"
41+
at evaluate path "/properties/contents/\$ref/additionalProperties/anyOf/1/\$ref"
42+
The object value was not expected to define the property "extends"
43+
at instance location "/contents/example/extends"
44+
at evaluate path "/properties/contents/\$ref/additionalProperties/anyOf/2/\$ref/additionalProperties"
45+
The object value was not expected to define additional properties
46+
at instance location "/contents/example"
47+
at evaluate path "/properties/contents/\$ref/additionalProperties/anyOf/2/\$ref/additionalProperties"
48+
The object value was expected to validate against the referenced schema
49+
at instance location "/contents/example"
50+
at evaluate path "/properties/contents/\$ref/additionalProperties/anyOf/2/\$ref"
51+
The object value was expected to validate against at least one of the 3 given subschemas
52+
at instance location "/contents/example"
53+
at evaluate path "/properties/contents/\$ref/additionalProperties/anyOf"
54+
The object properties not covered by other adjacent object keywords were expected to validate against this subschema
55+
at instance location "/contents"
56+
at evaluate path "/properties/contents/\$ref/additionalProperties"
57+
The object value was expected to validate against the referenced schema
58+
at instance location "/contents"
59+
at evaluate path "/properties/contents/\$ref"
60+
The object value was expected to validate against the 5 defined properties subschemas
61+
at instance location ""
62+
at evaluate path "/properties"
63+
EOF
64+
65+
diff "$TMP/output.txt" "$TMP/expected.txt"

0 commit comments

Comments
 (0)