Conversation
Signed-off-by: Juan Cruz Viotti <[email protected]>
🤖 Augment PR SummarySummary: Refreshes vendored Sourcemeta dependencies and aligns URI/JSON handling with newer Changes:
Notes: URI component encoding/decoding semantics are materially different; downstream code should be validated for expected round-trip behavior. 🤖 Was this summary useful? React with 👍 or 👎 |
| 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 compiled out in release builds, but the code still calls stream.write(...) even if std::to_chars fails (e.g., NaN/Inf), which could emit incorrect output. Consider ensuring non-success result.ec is handled so invalid JSON isn’t produced silently.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| if (this->path_.has_value()) { | ||
| uri_normalize_percent_encoding_inplace(this->path_.value()); | ||
| uri_unescape_if_inplace(this->path_.value(), is_pchar); |
There was a problem hiding this comment.
Unescaping percent-encoded : in path_ during canonicalize() can turn a valid relative-ref (where : was encoded to satisfy RFC3986 path-noscheme) into an output that re-parses differently (or as a scheme). Worth confirming canonicalization should preserve %3A in the first segment when scheme_/authority are absent to keep round-trip semantics.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| uri_unescape_selective_inplace(parsed_path.value()); | ||
| uri_unescape_unreserved_inplace(parsed_path.value()); |
There was a problem hiding this comment.
Switching to uri_unescape_unreserved_inplace means URI::path()/query()/etc now retain percent-encoded reserved chars and spaces (e.g., %20) rather than fully decoding as before, which is a public behavioral change for callers. Worth ensuring downstream code/tests/docs expect components to remain percent-encoded except for unreserved.
Severity: medium
Other Locations
vendor/core/src/core/uri/parse.cc:459vendor/core/src/core/uri/parse.cc:464vendor/core/src/core/uri/parse.cc:590vendor/core/src/core/uri/parse.cc:596
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Benchmark Index (community)
Details
| Benchmark suite | Current: d0003db | Previous: 162be09 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
19 ms |
21 ms |
0.90 |
Add one schema (100 existing) |
24 ms |
26 ms |
0.92 |
Add one schema (1000 existing) |
82 ms |
86 ms |
0.95 |
Add one schema (10000 existing) |
652 ms |
729 ms |
0.89 |
Update one schema (1 existing) |
17 ms |
18 ms |
0.94 |
Update one schema (101 existing) |
24 ms |
26 ms |
0.92 |
Update one schema (1001 existing) |
77 ms |
86 ms |
0.90 |
Update one schema (10001 existing) |
657 ms |
735 ms |
0.89 |
Cached rebuild (1 existing) |
10 ms |
10 ms |
1 |
Cached rebuild (101 existing) |
12 ms |
16 ms |
0.75 |
Cached rebuild (1001 existing) |
33 ms |
37 ms |
0.89 |
Cached rebuild (10001 existing) |
264 ms |
299 ms |
0.88 |
Index 100 schemas |
110 ms |
125 ms |
0.88 |
Index 1000 schemas |
900 ms |
1128 ms |
0.80 |
Index 10000 schemas |
15089 ms |
13919 ms |
1.08 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Benchmark Index (enterprise)
Details
| Benchmark suite | Current: d0003db | Previous: 162be09 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
18 ms |
20 ms |
0.90 |
Add one schema (100 existing) |
23 ms |
26 ms |
0.88 |
Add one schema (1000 existing) |
64 ms |
76 ms |
0.84 |
Add one schema (10000 existing) |
597 ms |
648 ms |
0.92 |
Update one schema (1 existing) |
18 ms |
20 ms |
0.90 |
Update one schema (101 existing) |
23 ms |
25 ms |
0.92 |
Update one schema (1001 existing) |
67 ms |
75 ms |
0.89 |
Update one schema (10001 existing) |
538 ms |
757 ms |
0.71 |
Cached rebuild (1 existing) |
11 ms |
11 ms |
1 |
Cached rebuild (101 existing) |
11 ms |
13 ms |
0.85 |
Cached rebuild (1001 existing) |
25 ms |
35 ms |
0.71 |
Cached rebuild (10001 existing) |
179 ms |
280 ms |
0.64 |
Index 100 schemas |
90 ms |
164 ms |
0.55 |
Index 1000 schemas |
789 ms |
1046 ms |
0.75 |
Index 10000 schemas |
11030 ms |
13248 ms |
0.83 |
This comment was automatically generated by workflow using github-action-benchmark.
Signed-off-by: Juan Cruz Viotti [email protected]