Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
🤖 Augment PR SummarySummary: Upgrades the vendored Sourcemeta Core dependency (and accompanying Blaze/Jsonbinpack updates) and refactors JSONSchema CLI code to use the new Core Changes:
Technical Notes: URI-template router binary format changes imply previously serialized router files must be regenerated using the new version. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| } | ||
|
|
||
| const auto canonical_path{sourcemeta::core::canonical(path)}; | ||
| std::ifstream stream{canonical_path}; |
There was a problem hiding this comment.
vendor/core/src/lang/io/include/sourcemeta/core/io.h:99: read_file is templated on CharT/Traits but constructs a std::ifstream, which ignores those template parameters and can break (or fail to compile) for non-char instantiations. This looks like it should construct std::basic_ifstream<CharT, Traits> to match the return type.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| std::basic_string<CharT, Traits> result; | ||
| result.resize( | ||
| static_cast<std::size_t>(std::filesystem::file_size(canonical_path))); |
There was a problem hiding this comment.
vendor/core/src/lang/io/include/sourcemeta/core/io.h:173: read_file_to_string relies on std::filesystem::file_size(), which will throw for non-regular files (e.g., FIFOs like /dev/fd/* from process substitution). Since canonical() explicitly preserves FIFO paths, this can regress previously-working FIFO reads (JSON/YAML parsing now routes through read_file_to_string).
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| std::string base_path_; | ||
| std::vector<std::pair<Identifier, std::vector<Argument>>> arguments_; | ||
| std::vector<std::tuple<Identifier, Identifier, std::string_view>> entries_; | ||
| std::unordered_map<std::string_view, std::pair<Identifier, Identifier>> |
There was a problem hiding this comment.
vendor/core/src/core/uritemplate/include/sourcemeta/core/uritemplate_router.h:149: operations_ uses std::string_view keys, so passing an operation_id that doesn’t outlive the router can leave dangling references and make operation() unsafe. The doc comment mentions string lifetime for uri_template but not for operation_id, which seems equally required given this storage.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| // `std::filesystem::rename` stays on a single filesystem and remains | ||
| // atomic. Using the system-wide temporary directory would risk `EXDEV` | ||
| // errors on cross-filesystem builds (CI containers, NFS mounts, etc.). | ||
| this->staging_ += ".tmp"; |
There was a problem hiding this comment.
vendor/core/src/lang/io/io_atomic.cc:32: Using a fixed staging name (destination + ".tmp") can collide if two writers target the same destination concurrently (or if a stale staging file exists), which can undermine the intended atomicity guarantees. Consider whether the staging filename needs to be unique per write attempt.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
jviotti
changed the title
ce85e618da1277977a08dcc360135e2fcbefe4724707860933842426b0e9d1647215921967077c2a
1 participant
Signed-off-by: Juan Cruz Viotti jv@jviotti.com