Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
There was a problem hiding this comment.
No issues found across 160 files
Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
🤖 Augment PR SummarySummary: This PR updates the repository’s vendored Sourcemeta stack (notably Changes:
Technical Notes: Several changes aim to make parsing/normalization non-throwing (avoiding 🤖 Was this summary useful? React with 👍 or 👎 |
| } else if ((static_cast<unsigned char>(character) & 0x80U) != 0U) { | ||
| // Accept the non-ASCII characters permitted by RFC 3987, so that IRI | ||
| // paths are handled in addition to URI paths | ||
| const auto decoded{sourcemeta::core::utf8_decode(output, index)}; |
There was a problem hiding this comment.
canonicalize_path now accepts literal RFC 3987 ucschar bytes, but it still only runs uri_unescape_unreserved_inplace, so percent-encoded UTF-8 for ucschar won’t normalize to the same form as the literal path. This can make URI::strip_path_prefix/rebase_path fail for equivalent IRI paths depending on whether they were encoded or literal.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| return result; | ||
| return result; | ||
| } catch (const YAMLParseError &error) { |
There was a problem hiding this comment.
Catching YAMLParseError here will also catch derived YAML error types and rethrow them as YAMLFileParseError, which may break callers that rely on distinguishing specific subclasses. If the intent is “add path context”, consider whether preserving the original dynamic type matters for your API surface.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| bool format_assertion{false}; | ||
| /// Select which keywords emit annotations in exhaustive mode. When not set, | ||
| /// every annotation keyword is emitted | ||
| std::optional<std::unordered_set<sourcemeta::core::JSON::StringView>> |
There was a problem hiding this comment.
Tweaks::annotations stores JSON::StringView inside an owning unordered_set, so if callers populate it from non-static/temporary strings the set can outlive the referenced storage and later lookups become undefined behavior. This seems like a footgun unless the required lifetime constraints are enforced or clearly documented.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| this->root_.variable != nullptr; | ||
| } | ||
|
|
||
| return true; |
There was a problem hiding this comment.
URITemplateRouter::describes() currently returns true for any path that can be walked to an existing node, even if the final node has identifier == 0 and no children (e.g., stale nodes after route replacement). That can cause describes() to report paths as described even though no registered route/prefix/capture actually matches them.
Severity: medium
Other Locations
vendor/core/src/core/uritemplate/uritemplate_router_view.cc:754
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Signed-off-by: Juan Cruz Viotti jv@jviotti.com