LibURL: Replace WTF-8 surrogates before Rust URL parsing
The Rust URL FFI used String::from_utf8_lossy() on raw bytes from C++. That was not equivalent to the old C++ parser behavior for WTF-8 encoded surrogates. For example a byte sequence such as ED A0 80 was treated as three invalid UTF-8 bytes, producing three U+FFFD replacement characters. Instead use String::from_utf8_with_replacement_character and let Rust borrow the input as &str.
This commit is contained in:
parent
854d9c7da4
commit
b4f46211d2
3 changed files with 36 additions and 13 deletions
|
|
@ -232,9 +232,8 @@ pub unsafe extern "C" fn rust_url_basic_parse(
|
|||
on_complete: FfiUrlResultFn,
|
||||
) -> bool {
|
||||
abort_on_panic(|| {
|
||||
// SAFETY: caller guarantees input and options are valid.
|
||||
let input_bytes = unsafe { std::slice::from_raw_parts(input, input_length) };
|
||||
let input_str = String::from_utf8_lossy(input_bytes);
|
||||
// SAFETY: caller guarantees input is scalar-value UTF-8 and valid for input_length bytes.
|
||||
let input_str = unsafe { std::str::from_utf8_unchecked(std::slice::from_raw_parts(input, input_length)) };
|
||||
|
||||
let options = unsafe { options.as_ref() };
|
||||
|
||||
|
|
@ -258,9 +257,9 @@ pub unsafe extern "C" fn rust_url_basic_parse(
|
|||
parse_options.base_url = base_url.as_ref();
|
||||
|
||||
let (did_succeed, maybe_url) = if let Some(mut existing_url) = existing_url {
|
||||
let did_succeed = basic_parse_into(&input_str, &mut existing_url, &parse_options);
|
||||
let did_succeed = basic_parse_into(input_str, &mut existing_url, &parse_options);
|
||||
(did_succeed, Some(existing_url))
|
||||
} else if let Some(parsed) = basic_parse(&input_str, parse_options) {
|
||||
} else if let Some(parsed) = basic_parse(input_str, parse_options) {
|
||||
(true, Some(parsed))
|
||||
} else {
|
||||
(false, None)
|
||||
|
|
@ -302,11 +301,10 @@ pub unsafe extern "C" fn rust_url_parse_host(
|
|||
on_complete: FfiHostResultFn,
|
||||
) -> bool {
|
||||
abort_on_panic(|| {
|
||||
// SAFETY: caller guarantees input is valid.
|
||||
let input_bytes = unsafe { std::slice::from_raw_parts(input, input_length) };
|
||||
let input_str = String::from_utf8_lossy(input_bytes);
|
||||
// SAFETY: caller guarantees input is scalar-value UTF-8 and valid for input_length bytes.
|
||||
let input_str = unsafe { std::str::from_utf8_unchecked(std::slice::from_raw_parts(input, input_length)) };
|
||||
|
||||
let Some(host) = parse_host(&input_str, is_opaque) else {
|
||||
let Some(host) = parse_host(input_str, is_opaque) else {
|
||||
// SAFETY: on_complete is a valid function pointer; ctx is caller-provided.
|
||||
unsafe { on_complete(ctx, std::ptr::null()) };
|
||||
return false;
|
||||
|
|
|
|||
|
|
@ -418,11 +418,15 @@ static void on_parse_host_complete(void* ctx_ptr, FFI::FfiUrlHost const* ffi_res
|
|||
|
||||
Optional<Host> parse_host(StringView input, bool is_opaque)
|
||||
{
|
||||
// URL parsing expects a scalar-value UTF-8 string, but WTF-8 can be provided.
|
||||
auto processed_input = String::from_utf8_with_replacement_character(input, String::WithBOMHandling::No);
|
||||
auto processed_input_view = processed_input.bytes_as_string_view();
|
||||
|
||||
Optional<Host> result;
|
||||
HostParseCallbackCtx ctx { .result = &result };
|
||||
bool const did_succeed = FFI::rust_url_parse_host(
|
||||
reinterpret_cast<uint8_t const*>(input.characters_without_null_termination()),
|
||||
input.length(),
|
||||
reinterpret_cast<uint8_t const*>(processed_input_view.characters_without_null_termination()),
|
||||
processed_input_view.length(),
|
||||
is_opaque,
|
||||
&ctx,
|
||||
on_parse_host_complete);
|
||||
|
|
@ -437,6 +441,9 @@ Optional<URL> parse_basic_url(StringView input, Optional<URL const&> base_url, U
|
|||
return static_cast<FFI::State>(to_underlying(state));
|
||||
};
|
||||
|
||||
// URL parsing expects a scalar-value UTF-8 string, but WTF-8 can be provided.
|
||||
auto processed_input = String::from_utf8_with_replacement_character(input, String::WithBOMHandling::No);
|
||||
|
||||
Optional<UrlFfiStorage> base_storage;
|
||||
if (base_url.has_value())
|
||||
base_storage = url_to_ffi(*base_url);
|
||||
|
|
@ -460,9 +467,10 @@ Optional<URL> parse_basic_url(StringView input, Optional<URL const&> base_url, U
|
|||
|
||||
Optional<URL> result;
|
||||
ParseCallbackCtx ctx { .result = &result, .url_inout = url };
|
||||
auto processed_input_view = processed_input.bytes_as_string_view();
|
||||
bool const did_succeed = rust_url_basic_parse(
|
||||
reinterpret_cast<uint8_t const*>(input.characters_without_null_termination()),
|
||||
input.length(),
|
||||
reinterpret_cast<uint8_t const*>(processed_input_view.characters_without_null_termination()),
|
||||
processed_input_view.length(),
|
||||
&options,
|
||||
&ctx,
|
||||
on_basic_parse_complete);
|
||||
|
|
|
|||
|
|
@ -374,6 +374,23 @@ TEST_CASE(unicode)
|
|||
EXPECT(!url->fragment().has_value());
|
||||
}
|
||||
|
||||
TEST_CASE(wtf8_surrogates_are_replaced_before_url_parsing)
|
||||
{
|
||||
{
|
||||
auto url = URL::Parser::basic_parse("http://example.com/\xED\xA0\x80-\xED\xB0\x80?\xED\xBF\xBF"sv);
|
||||
EXPECT(url.has_value());
|
||||
EXPECT_EQ(url->serialize_path(), "/%EF%BF%BD-%EF%BF%BD");
|
||||
EXPECT_EQ(url->query(), "%EF%BF%BD");
|
||||
EXPECT_EQ(url->serialize(), "http://example.com/%EF%BF%BD-%EF%BF%BD?%EF%BF%BD");
|
||||
}
|
||||
|
||||
{
|
||||
auto host = URL::Parser::parse_host("\xED\xA0\x80host\xED\xBF\xBF"sv, true);
|
||||
EXPECT(host.has_value());
|
||||
EXPECT_EQ(host->serialize(), "%EF%BF%BDhost%EF%BF%BD");
|
||||
}
|
||||
}
|
||||
|
||||
TEST_CASE(query_with_non_ascii)
|
||||
{
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue