[strings] Prevent OOM crash in URI encoding [v8/v8 : main]
0 views
Skip to first unread message
Leszek Swirski (Gerrit)
Nov 17, 2025, 9:58:10 AM (2 days ago) Nov 17
to Igor Sheludko, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Igor Sheludko
Leszek Swirski added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Igor Sheludko
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Igor Sheludko (Gerrit)
Nov 17, 2025, 10:48:35 AM (2 days ago) Nov 17
to Leszek Swirski, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Igor Sheludko voted and added 5 comments![]( "Open in Gerrit")
Votes added by Igor Sheludko
| Code-Review | +1 |
5 comments
Patchset-level comments
Igor Sheludko . resolved
lgtm with nits
File src/strings/uri.cc
Line 345, Patchset 1 (Latest):
int uri_length = uri->length();Igor Sheludko . unresolved
While you are here, nit: `uint32_t`.
Line 347, Patchset 1 (Latest):
base::uc16 cc1 = uri_content.Get(k);Igor Sheludko . unresolved
Nit: we are checking if the encoding of the string is one byte for every character. Please consider adding a TODO to optimize this.
Line 348, Patchset 1 (Latest):
if (unibrow::Utf16::IsLeadSurrogate(cc1)) {Igor Sheludko . unresolved
Nit: is this necessary if the uri is a one-byte string? TODO!
Related details
Attention is currently required from:
- Leszek Swirski
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Nov 17, 2025, 11:00:28 AM (2 days ago) Nov 17
to Igor Sheludko, V8 LUCI CQ, v8-re...@googlegroups.com
Leszek Swirski voted and added 5 comments![]( "Open in Gerrit")
Votes added by Leszek Swirski
| Auto-Submit | +1 |
5 comments
Patchset-level comments
File src/strings/uri.cc
Line 238, Patchset 1:
bool TryPushBack(Args... args) {Igor Sheludko . resolved
Leszek SwirskiNICE!!
😊
While you are here, nit: `uint32_t`.
Leszek Swirski
Obsolete with the one/two byte helper functions.
Nit: we are checking if the encoding of the string is one byte for every character. Please consider adding a TODO to optimize this.
Leszek Swirski
Why TODO when I can just DO 😊
Nit: is this necessary if the uri is a one-byte string? TODO!
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Igor Sheludko (Gerrit)
Nov 17, 2025, 11:31:46 AM (2 days ago) Nov 17
to Leszek Swirski, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Igor Sheludko voted and added 1 comment![]( "Open in Gerrit")
Votes added by Igor Sheludko
| Code-Review | +1 |
1 comment
Patchset-level comments
Igor Sheludko . resolved
lgtm
Attention is currently required from:
- Leszek Swirski
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Nov 18, 2025, 8:25:16 AM (yesterday) Nov 18
to Igor Sheludko, V8 LUCI CQ, v8-re...@googlegroups.com
Leszek Swirski voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Nov 18, 2025, 8:26:51 AM (yesterday) Nov 18
to Leszek Swirski, Igor Sheludko, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[strings] Prevent OOM crash in URI encoding
The std::vector used as a buffer in EncodeURI can fail to allocate when
encoding very large strings. This ends up OOMing and crashing the
process.
This patch replaces the std::vector with a custom ResizableBuffer that
handles allocation failures gracefully. This allows the builtin to stop
processing and throw a InvalidStringLengthError instead of crashing.
Since this is a new failure mode, additionally do some cleanup of the
encode method to use a non-allocating helper which returns a status.
Bug: 459950607
Change-Id: I8d580832297b2c16c8b651c2f691e916c56d6390
Auto-Submit: Leszek Swirski <les...@chromium.org>
Reviewed-by: Igor Sheludko <ish...@chromium.org>
Commit-Queue: Leszek Swirski <les...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#103788}
Files:
- M src/strings/uri.cc
Change size: M
Delta: 1 file changed, 147 insertions(+), 59 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Igor Sheludko
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages