Skip to content

Commit fc61f15

Browse files
jviottijviotti
authored
Fix infinite cycles in configuration includes/extends (#775)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent 5f7a697 commit fc61f15

12 files changed

Lines changed: 156 additions & 15 deletions

‎src/configuration/include/sourcemeta/one/configuration_error.h‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55

66
#include <sourcemeta/core/jsonpointer.h>
77

8-
#include <exception> // std::exception
9-
#include <sstream> // std::ostringstream
10-
#include <string> // std::string
8+
#include <exception> // std::exception
9+
#include <filesystem> // std::filesystem::path
10+
#include <sstream> // std::ostringstream
11+
#include <string> // std::string
12+
#include <utility> // std::move
1113

1214
namespace sourcemeta::one {
1315

@@ -99,6 +101,37 @@ class ConfigurationUnknownBuiltInCollectionError : public std::exception {
99101
std::string identifier_;
100102
};
101103

104+
class ConfigurationCyclicReferenceError : public std::exception {
105+
public:
106+
ConfigurationCyclicReferenceError(std::filesystem::path from,
107+
sourcemeta::core::Pointer location,
108+
std::filesystem::path target)
109+
: from_{std::move(from)}, location_{std::move(location)},
110+
target_{std::move(target)} {}
111+
112+
[[nodiscard]] auto what() const noexcept -> const char * override {
113+
return "Circular reference detected in configuration";
114+
}
115+
116+
[[nodiscard]] auto from() const noexcept -> const std::filesystem::path & {
117+
return this->from_;
118+
}
119+
120+
[[nodiscard]] auto target() const noexcept -> const std::filesystem::path & {
121+
return this->target_;
122+
}
123+
124+
[[nodiscard]] auto location() const noexcept
125+
-> const sourcemeta::core::Pointer & {
126+
return this->location_;
127+
}
128+
129+
private:
130+
std::filesystem::path from_;
131+
sourcemeta::core::Pointer location_;
132+
std::filesystem::path target_;
133+
};
134+
102135
} // namespace sourcemeta::one
103136

104137
#endif

‎src/configuration/read.cc‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
#include <sourcemeta/one/configuration.h>
22

3-
#include <algorithm> // std::ranges
4-
#include <cassert> // assert
5-
#include <iterator> // std::back_inserter
6-
#include <utility> // std::move
7-
#include <vector> // std::vector
3+
#include <algorithm> // std::ranges
4+
#include <cassert> // assert
5+
#include <iterator> // std::back_inserter
6+
#include <string> // std::string
7+
#include <unordered_set> // std::unordered_set
8+
#include <utility> // std::move
9+
#include <vector> // std::vector
810

911
namespace {
1012

@@ -49,7 +51,8 @@ auto maybe_suffix(const std::filesystem::path &path,
4951
auto dereference(const std::filesystem::path &collections_path,
5052
const std::filesystem::path &base,
5153
sourcemeta::core::JSON &input,
52-
const sourcemeta::core::Pointer &location) -> void {
54+
const sourcemeta::core::Pointer &location,
55+
std::unordered_set<std::string> &visited) -> void {
5356
assert(base.is_absolute());
5457
if (!input.is_object()) {
5558
return;
@@ -64,20 +67,27 @@ auto dereference(const std::filesystem::path &collections_path,
6467
entry.to_string()),
6568
"one.json")};
6669
const auto new_location{location.concat({"extends"})};
70+
if (!visited.emplace(target_path.native()).second) {
71+
throw sourcemeta::one::ConfigurationCyclicReferenceError(
72+
base, new_location, target_path);
73+
}
6774
auto extension{
6875
read_file(base, new_location, target_path, entry.to_string())};
6976
if (extension.is_object()) {
70-
dereference(collections_path, target_path, extension, new_location);
77+
dereference(collections_path, target_path, extension, new_location,
78+
visited);
7179
accumulator.merge(std::move(extension).as_object());
7280
}
81+
82+
visited.erase(target_path.native());
7383
}
7484
}
7585

7686
input.erase("extends");
7787
accumulator.merge(input.as_object());
7888
input = std::move(accumulator);
7989
assert(!input.defines("extends"));
80-
dereference(collections_path, base, input, location);
90+
dereference(collections_path, base, input, location, visited);
8191

8292
// Read included files
8393
} else if (!location.empty() && input.defines("include") &&
@@ -89,10 +99,14 @@ auto dereference(const std::filesystem::path &collections_path,
8999
input.at("include").to_string()),
90100
"jsonschema.json")};
91101
const auto new_location{location.concat({"include"})};
102+
if (!visited.emplace(target_path.native()).second) {
103+
throw sourcemeta::one::ConfigurationCyclicReferenceError(
104+
base, new_location, target_path);
105+
}
92106
input.into(read_file(base, new_location, target_path,
93107
input.at("include").to_string()));
94-
assert(!input.defines("include"));
95-
dereference(collections_path, target_path, input, new_location);
108+
dereference(collections_path, target_path, input, new_location, visited);
109+
visited.erase(target_path.native());
96110

97111
// Revisit and relativize paths
98112
} else if (input.defines("path") && input.at("path").is_string()) {
@@ -116,7 +130,7 @@ auto dereference(const std::filesystem::path &collections_path,
116130
[](const auto &entry) { return entry.first; });
117131
for (const auto &key : keys) {
118132
dereference(collections_path, base, input.at("contents").at(key),
119-
location.concat({"contents", key}));
133+
location.concat({"contents", key}), visited);
120134
}
121135
}
122136
}
@@ -168,7 +182,10 @@ auto Configuration::read(const std::filesystem::path &configuration_path,
168182
sourcemeta::core::JSON{"The next-generation JSON Schema platform"});
169183
}
170184

171-
dereference(collections_path, configuration_path, data, {});
185+
std::unordered_set<std::string> visited;
186+
visited.emplace(
187+
std::filesystem::weakly_canonical(configuration_path).native());
188+
dereference(collections_path, configuration_path, data, {}, visited);
172189

173190
if (data.is_object() && data.defines("url") && data.defines("contents") &&
174191
data.at("contents").is_object()) {

‎src/index/index.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,13 @@ auto main(int argc, char *argv[]) noexcept -> int {
484484
const std::string_view program{argv[0]};
485485

486486
return index_main(program, app);
487+
} catch (const sourcemeta::one::ConfigurationCyclicReferenceError &error) {
488+
std::cerr << "error: " << error.what() << "\n";
489+
std::cerr << " from " << error.from().string() << "\n";
490+
std::cerr << " at \"" << sourcemeta::core::to_string(error.location())
491+
<< "\"\n";
492+
std::cerr << " to " << error.target().string() << "\n";
493+
return EXIT_FAILURE;
487494
} catch (const sourcemeta::one::ConfigurationReadError &error) {
488495
std::cerr << "error: " << error.what() << "\n";
489496
std::cerr << " from " << error.from().string() << "\n";

‎test/unit/configuration/configuration_read_test.cc‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,3 +512,51 @@ TEST(Configuration_read, read_invalid_003) {
512512
FAIL();
513513
}
514514
}
515+
516+
TEST(Configuration_read, read_valid_016_diamond_extends) {
517+
const auto configuration_path{std::filesystem::path{STUB_DIRECTORY} /
518+
"read_valid_016.json"};
519+
const auto result{sourcemeta::one::Configuration::read(
520+
configuration_path, COLLECTIONS_DIRECTORY)};
521+
EXPECT_TRUE(result.is_object());
522+
EXPECT_TRUE(result.defines("contents"));
523+
EXPECT_TRUE(result.at("contents").defines("shared"));
524+
EXPECT_TRUE(result.at("contents").defines("from_b"));
525+
EXPECT_TRUE(result.at("contents").defines("from_c"));
526+
}
527+
528+
TEST(Configuration_read, read_invalid_004_circular_extends) {
529+
const auto configuration_path{std::filesystem::path{STUB_DIRECTORY} /
530+
"read_invalid_004.json"};
531+
532+
try {
533+
sourcemeta::one::Configuration::read(configuration_path,
534+
COLLECTIONS_DIRECTORY);
535+
FAIL();
536+
} catch (const sourcemeta::one::ConfigurationCyclicReferenceError &error) {
537+
EXPECT_EQ(error.target(), std::filesystem::weakly_canonical(
538+
std::filesystem::path{STUB_DIRECTORY} /
539+
"read_invalid_004.json"));
540+
EXPECT_EQ(sourcemeta::core::to_string(error.location()),
541+
"/extends/extends");
542+
} catch (...) {
543+
FAIL();
544+
}
545+
}
546+
547+
TEST(Configuration_read, read_invalid_006_circular_include) {
548+
const auto configuration_path{std::filesystem::path{STUB_DIRECTORY} /
549+
"read_invalid_006.json"};
550+
551+
try {
552+
sourcemeta::one::Configuration::read(configuration_path,
553+
COLLECTIONS_DIRECTORY);
554+
FAIL();
555+
} catch (const sourcemeta::one::ConfigurationCyclicReferenceError &error) {
556+
EXPECT_EQ(error.target(), std::filesystem::weakly_canonical(
557+
std::filesystem::path{STUB_DIRECTORY} /
558+
"read_invalid_006.json"));
559+
} catch (...) {
560+
FAIL();
561+
}
562+
}

‎test/unit/configuration/stub/read_invalid_004.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{ "extends": [ "./read_invalid_005.json" ] }

‎test/unit/configuration/stub/read_invalid_005.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{ "extends": [ "./read_invalid_004.json" ] }

‎test/unit/configuration/stub/read_invalid_006.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"url": "https://sourcemeta.com/",
3+
"contents": {
4+
"x": { "include": "./read_invalid_007.json" }
5+
}
6+
}

‎test/unit/configuration/stub/read_invalid_007.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{ "include": "./read_invalid_006.json" }

‎test/unit/configuration/stub/read_valid_016.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"url": "http://localhost:8000",
3+
"extends": [ "./read_valid_016_b.json", "./read_valid_016_c.json" ]
4+
}

‎test/unit/configuration/stub/read_valid_016_b.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"extends": [ "./read_valid_016_d.json" ],
3+
"contents": {
4+
"from_b": {
5+
"title": "From B"
6+
}
7+
}
8+
}

0 commit comments

Comments
 (0)