Upgrade Core to 56eab6ef118e9731df539d3a507db1c23241f911#908
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: This PR updates the vendored Changes:
Technical Notes: The URI changes aim to better align percent-encoding behavior with RFC 3986 component grammar and avoid incorrect escaping (e.g., treating path as fragment). 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| const auto result{ | ||
| std::to_chars(buffer.data(), buffer.data() + buffer.size(), value)}; | ||
| // This can't realistically happen on production given the buffer size | ||
| assert(result.ec == std::errc{}); |
There was a problem hiding this comment.
assert(result.ec == std::errc{}) is the only guard here; when assertions are disabled, a std::to_chars failure (e.g., NaN/Inf or unexpected buffer exhaustion) could still flow into stream.write(...) with an unspecified result.ptr. Consider handling result.ec explicitly (or falling back) to avoid UB/invalid output in release builds.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| if (this->query_.has_value()) { | ||
| uri_normalize_percent_encoding_inplace(this->query_.value()); | ||
| uri_unescape_if_inplace(this->query_.value(), [&](char character) { | ||
| return is_pchar(character) || character == URI_SLASH || |
There was a problem hiding this comment.
In canonicalize(), unescaping percent-encoded reserved characters in the query/fragment (via is_pchar including sub-delims, plus /?) can change URI semantics (e.g., %26 → & in query strings). RFC 3986 normalization typically only decodes percent-encoded unreserved characters, so it may be worth double-checking the intended behavior here.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| template <typename... Args> | ||
| FileError(std::filesystem::path path, Args &&...args) | ||
| : T{std::forward<Args>(args)...}, path_{std::move(path)} { | ||
| assert(std::filesystem::exists(this->path_)); |
There was a problem hiding this comment.
The constructor asserts std::filesystem::exists(this->path_), which can fire in debug builds for common error cases like “file not found” / permission issues where you still want to attach the path. If FileError is meant to wrap I/O failures, this precondition may be too strict.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
1 participant
Signed-off-by: Juan Cruz Viotti jv@jviotti.com