From c438976e4b56c0f0a7ab4069eff39998c70e0045 Mon Sep 17 00:00:00 2001 From: Claudio DeSouza Date: Wed, 27 Nov 2024 13:54:48 +0000 Subject: [PATCH] [Spanify] `span` size/offset to conform to `size_t` There's some tightening around values passed into span, with `StrictNumeric` being enforced. This is mostly unimportant when dealing with compile-time values, as they can be validated on the spot, however it does affect several places where the value is unknown at runtime, and a signed value was being used. Chromium changes: https://chromium.googlesource.com/chromium/src/+/4937ad9ae64d9d3f05d13c9d041769432a12281b ``` commit 4937ad9ae64d9d3f05d13c9d041769432a12281b Author: Peter Kasting Date: Tue Nov 26 22:58:16 2024 +0000 Use StrictNumeric for more span methods. Changes subspan(), split_at(), at(), and get_at() to match pre-existing usage in constructors, first(), last(), etc. Bug: 364987728 ``` Resolves https://github.com/brave/brave-browser/issues/42627 --- browser/ui/views/tabs/tab_drag_controller.cc | 3 ++- .../recent_tabs_sub_menu_model_browsertest.cc | 2 +- .../brave_wallet/browser/eth_data_parser.cc | 4 ++-- .../browser/json_rpc_service_unittest.cc | 14 +++++++------- .../brave_wallet/browser/sns_resolver_task.cc | 12 ++++++------ .../brave_wallet/common/brave_wallet_constants.h | 10 +++++----- components/brave_wallet/common/eth_abi_utils.cc | 16 ++++++++-------- components/brave_wallet/common/zcash_utils.cc | 10 +++++----- components/ipfs/ipfs_utils.cc | 2 +- components/tor/tor_control.cc | 10 +++++----- components/tor/tor_control.h | 2 +- components/tor/tor_control_unittest.cc | 16 ++++++++-------- 12 files changed, 51 insertions(+), 50 deletions(-) diff --git a/browser/ui/views/tabs/tab_drag_controller.cc b/browser/ui/views/tabs/tab_drag_controller.cc index d64497360e50..fdb9b1d88b25 100644 --- a/browser/ui/views/tabs/tab_drag_controller.cc +++ b/browser/ui/views/tabs/tab_drag_controller.cc @@ -187,7 +187,8 @@ void TabDragController::DetachAndAttachToNewContext( std::vector tabs; auto* tab_strip_model = browser->tab_strip_model(); DCHECK_EQ(tab_strip_model, attached_context_->GetTabStripModel()); - auto drag_data = base::span(drag_data_).subspan(first_tab_index()); + auto drag_data = + base::span(drag_data_).subspan(static_cast(first_tab_index())); for (const auto& data : drag_data) { tabs.push_back(tab_strip_model->GetTabHandleAt( tab_strip_model->GetIndexOfWebContents(data.contents))); diff --git a/chromium_src/chrome/browser/ui/tabs/recent_tabs_sub_menu_model_browsertest.cc b/chromium_src/chrome/browser/ui/tabs/recent_tabs_sub_menu_model_browsertest.cc index 0b2a5cf5ca81..08b09e4b0cdd 100644 --- a/chromium_src/chrome/browser/ui/tabs/recent_tabs_sub_menu_model_browsertest.cc +++ b/chromium_src/chrome/browser/ui/tabs/recent_tabs_sub_menu_model_browsertest.cc @@ -80,7 +80,7 @@ void RecentTabsSubMenuModelTest::VerifyModel( // The first two commands are History and History Clusters, but we disable // History Clusters and upstream won't show it, so we should skip one command. - ::VerifyModel(model, base::make_span(data).subspan(1)); + ::VerifyModel(model, base::span(data).subspan(1u)); } void RecentTabsSubMenuModelTest::VerifyModel(const ui::MenuModel* model, diff --git a/components/brave_wallet/browser/eth_data_parser.cc b/components/brave_wallet/browser/eth_data_parser.cc index 7d220d863d7f..b228449e7fdd 100644 --- a/components/brave_wallet/browser/eth_data_parser.cc +++ b/components/brave_wallet/browser/eth_data_parser.cc @@ -267,7 +267,7 @@ std::optional SquidDecodeCall(const base::Value::List& call) { } auto [selector_span, calldata] = - base::span(*calldata_with_selector).split_at(4); + base::span(*calldata_with_selector).split_at<4>(); auto selector = ToHex(selector_span); @@ -544,7 +544,7 @@ GetTransactionInfoFromData(const std::vector& data) { std::vector(), nullptr); } - auto [selector_span, calldata] = base::span(data).split_at(4); + auto [selector_span, calldata] = base::span(data).split_at<4>(); std::string selector = ToHex(selector_span); if (selector == kFilForwarderTransferSelector) { diff --git a/components/brave_wallet/browser/json_rpc_service_unittest.cc b/components/brave_wallet/browser/json_rpc_service_unittest.cc index 5aab40941f2a..861e1426cb5e 100644 --- a/components/brave_wallet/browser/json_rpc_service_unittest.cc +++ b/components/brave_wallet/browser/json_rpc_service_unittest.cc @@ -410,7 +410,7 @@ class GetAccountInfoHandler : public SolRpcCallHandler { static std::vector MakeMintData(int supply) { std::vector data(82); - base::span(data).subspan(36).first<8u>().copy_from( + base::span(data).subspan(36u).first<8u>().copy_from( base::U64ToLittleEndian(supply)); return data; } @@ -421,10 +421,10 @@ class GetAccountInfoHandler : public SolRpcCallHandler { std::vector result(96 + data.size()); auto result_span = base::span(result); // Header. - base::ranges::copy(owner.bytes(), result_span.subspan(32, 32).begin()); + result_span.subspan<32, 32>().copy_from(owner.bytes()); // Data. - base::ranges::copy(data, result_span.subspan(96).begin()); + result_span.subspan(96u).copy_from(data); return result; } @@ -436,7 +436,7 @@ class GetAccountInfoHandler : public SolRpcCallHandler { std::vector result(32 + 64); // payload_address + signature. auto result_span = base::span(result); - base::ranges::copy(sol_record_payload_address.bytes(), result_span.begin()); + result_span.copy_prefix_from(sol_record_payload_address.bytes()); std::vector message; message.insert(message.end(), sol_record_payload_address.bytes().begin(), @@ -444,7 +444,7 @@ class GetAccountInfoHandler : public SolRpcCallHandler { message.insert(message.end(), sol_record_address.bytes().begin(), sol_record_address.bytes().end()); std::string hex_message = base::ToLowerASCII(base::HexEncode(message)); - ED25519_sign(result_span.subspan(32).data(), + ED25519_sign(result_span.subspan(32u).data(), reinterpret_cast(hex_message.data()), hex_message.length(), signer_key.data()); @@ -631,11 +631,11 @@ class GetProgramAccountsHandler : public SolRpcCallHandler { expected_filters.Append(base::Value::Dict()); expected_filters.back().GetDict().SetByDottedPath("memcmp.offset", 0); expected_filters.back().GetDict().SetByDottedPath( - "memcmp.bytes", Base58Encode(data_span.subspan(0, 32))); + "memcmp.bytes", Base58Encode(data_span.first<32>())); expected_filters.Append(base::Value::Dict()); expected_filters.back().GetDict().SetByDottedPath("memcmp.offset", 64); expected_filters.back().GetDict().SetByDottedPath( - "memcmp.bytes", Base58Encode(data_span.subspan(64, 1))); + "memcmp.bytes", Base58Encode(data_span.subspan(64u, 1u))); expected_filters.Append(base::Value::Dict()); expected_filters.back().GetDict().Set("dataSize", 165); diff --git a/components/brave_wallet/browser/sns_resolver_task.cc b/components/brave_wallet/browser/sns_resolver_task.cc index d5e365de4166..1814cb8718cd 100644 --- a/components/brave_wallet/browser/sns_resolver_task.cc +++ b/components/brave_wallet/browser/sns_resolver_task.cc @@ -142,7 +142,7 @@ base::span ExtractSpan(base::span& data, if (data.size() < size) { return {}; } - auto result = data.subspan(0, size); + auto result = data.first(size); data = data.subspan(size); return result; } @@ -239,8 +239,8 @@ std::optional ParseAndVerifySolRecordV1Data( } // Extract 32 bytes of address followed by 64 bytes of signature. - auto sol_record_payload_address = SolanaAddress::FromBytes( - sol_record_payload.subspan(0, kSolanaPubkeySize)); + auto sol_record_payload_address = + SolanaAddress::FromBytes(sol_record_payload.first()); if (!sol_record_payload_address) { return std::nullopt; } @@ -588,15 +588,15 @@ std::optional NameRegistryState::FromBytes( // 96 bytes of header block followed by data block(possibly empty). result.emplace(); result->parent_name = - *SolanaAddress::FromBytes(data_span.subspan(0, kSolanaPubkeySize)); + *SolanaAddress::FromBytes(data_span.first()); data_span = data_span.subspan(kSolanaPubkeySize); result->owner = - *SolanaAddress::FromBytes(data_span.subspan(0, kSolanaPubkeySize)); + *SolanaAddress::FromBytes(data_span.first()); data_span = data_span.subspan(kSolanaPubkeySize); result->data_class = - *SolanaAddress::FromBytes(data_span.subspan(0, kSolanaPubkeySize)); + *SolanaAddress::FromBytes(data_span.first()); data_span = data_span.subspan(kSolanaPubkeySize); result->data.assign(data_span.begin(), data_span.end()); diff --git a/components/brave_wallet/common/brave_wallet_constants.h b/components/brave_wallet/common/brave_wallet_constants.h index 50f141fd62e6..d552074e7963 100644 --- a/components/brave_wallet/common/brave_wallet_constants.h +++ b/components/brave_wallet/common/brave_wallet_constants.h @@ -8,11 +8,11 @@ namespace brave_wallet { -inline constexpr int kSolanaKeypairSize = 64; -inline constexpr int kSolanaSignatureSize = 64; -inline constexpr int kSolanaPrikeySize = 32; -inline constexpr int kSolanaPubkeySize = 32; -inline constexpr int kSolanaHashSize = 32; +inline constexpr size_t kSolanaKeypairSize = 64; +inline constexpr size_t kSolanaSignatureSize = 64; +inline constexpr size_t kSolanaPrikeySize = 32; +inline constexpr size_t kSolanaPubkeySize = 32; +inline constexpr size_t kSolanaHashSize = 32; // 1232 = 1280(IPv6 minimum MTU) - 40(size of the IPv6 header) - 8(size of the // fragment header) inline constexpr size_t kSolanaMaxTxSize = 1232; diff --git a/components/brave_wallet/common/eth_abi_utils.cc b/components/brave_wallet/common/eth_abi_utils.cc index 7f75bb29aaad..d655f83fd935 100644 --- a/components/brave_wallet/common/eth_abi_utils.cc +++ b/components/brave_wallet/common/eth_abi_utils.cc @@ -13,6 +13,7 @@ #include "base/check.h" #include "base/check_op.h" #include "base/containers/span.h" +#include "base/containers/to_vector.h" #include "base/notreached.h" #include "base/ranges/algorithm.h" #include "brave/components/brave_wallet/common/brave_wallet_types.h" @@ -102,7 +103,7 @@ std::optional ExtractHeadFromTuple(Span data, size_t tuple_pos) { } EthAddress ExtractAddress(Span32 address_encoded) { - return EthAddress::FromBytes(address_encoded.subspan(12)); + return EthAddress::FromBytes(address_encoded.subspan<12>()); } } // namespace @@ -114,7 +115,7 @@ std::pair ExtractFunctionSelectorAndArgsFromCall(Span data) { if ((data.size() - 4) % kRowLength) { return {}; } - return {data.subspan(0, 4), data.subspan(4)}; + return {data.first<4>(), data.subspan(4u)}; } std::pair, Span> ExtractArrayInfo(Span data) { @@ -167,8 +168,7 @@ std::optional> ExtractBytes(Span bytes_encoded) { if (!CheckPadding(padded_bytes_data, *bytes_len)) { return std::nullopt; } - Span bytes_result = padded_bytes_data.subspan(0, *bytes_len); - return std::vector{bytes_result.begin(), bytes_result.end()}; + return base::ToVector(padded_bytes_data.first(*bytes_len)); } std::optional ExtractString(Span string_encoded) { @@ -194,8 +194,8 @@ std::optional ExtractString(Span string_encoded) { return std::nullopt; } - Span string_result = padded_string_data.subspan(0, *string_len); - return std::string{string_result.begin(), string_result.end()}; + return std::string( + base::as_string_view(padded_string_data.first(*string_len))); } std::optional> ExtractStringArray(Span string_array) { @@ -367,11 +367,11 @@ ExtractFixedBytesFromTuple(Span data, size_t fixed_size, size_t tuple_pos) { return std::nullopt; } - if (!CheckPadding(head->subspan(0), fixed_size)) { + if (!CheckPadding(*head, fixed_size)) { return std::nullopt; } - return std::vector{head->begin(), head->begin() + fixed_size}; + return base::ToVector(head->first(fixed_size)); } // NOLINTNEXTLINE(runtime/references) diff --git a/components/brave_wallet/common/zcash_utils.cc b/components/brave_wallet/common/zcash_utils.cc index c616e8c350d4..dd8d7cc46bcf 100644 --- a/components/brave_wallet/common/zcash_utils.cc +++ b/components/brave_wallet/common/zcash_utils.cc @@ -53,16 +53,16 @@ std::optional ReadCompactSize(base::span& data) { uint8_t type = data[0]; if (data.size() > 0 && data[0] < 253) { value = type; - data = data.subspan(1); + data = data.subspan(1u); } else if (type == 253 && data.size() >= 3) { value = base::numerics::U16FromBigEndian(data.subspan<1, 2u>()); - data = data.subspan(1 + 2); + data = data.subspan(1u + 2); } else if (type <= 254 && data.size() >= 5) { value = base::numerics::U32FromBigEndian(data.subspan<1, 4u>()); - data = data.subspan(1 + 4); + data = data.subspan(1u + 4); } else if (data.size() >= 9) { value = base::numerics::U64FromBigEndian(data.subspan<1, 8u>()); - data = data.subspan(1 + 8); + data = data.subspan(1u + 8); } else { return std::nullopt; } @@ -97,7 +97,7 @@ std::optional> ParseUnifiedAddressBody( addr.second = std::vector(dejumbled_data.begin(), dejumbled_data.begin() + *size); result.push_back(std::move(addr)); - dejumbled_data = dejumbled_data.subspan(*size); + dejumbled_data = dejumbled_data.subspan(base::checked_cast(*size)); } return result; } diff --git a/components/ipfs/ipfs_utils.cc b/components/ipfs/ipfs_utils.cc index abf2d2f96930..56bef9a28027 100644 --- a/components/ipfs/ipfs_utils.cc +++ b/components/ipfs/ipfs_utils.cc @@ -47,7 +47,7 @@ base::span DecodeVarInt(base::span from, shift += 7; } while (*it++ & 0x80); *into = static_cast(ret); - return from.subspan(it - from.begin()); + return from.subspan(static_cast(std::distance(from.begin(), it))); } // Extracts cid and path from ipfs URLs like: diff --git a/components/tor/tor_control.cc b/components/tor/tor_control.cc index dc714d9f9c0d..631122016043 100644 --- a/components/tor/tor_control.cc +++ b/components/tor/tor_control.cc @@ -97,7 +97,7 @@ TorControl::TorControl(base::WeakPtr delegate, io_task_runner_(task_runner), writing_(false), reading_(false), - read_start_(-1), + read_start_(0u), read_cr_(false), delegate_(delegate) { DCHECK_CALLED_ON_VALID_SEQUENCE(owner_sequence_checker_); @@ -730,7 +730,7 @@ void TorControl::StartRead() { DCHECK(!cmdq_.empty() || !async_events_.empty()); readiobuf_ = base::MakeRefCounted(); readiobuf_->SetCapacity(kTorBufferSize); - read_start_ = 0; + read_start_ = 0u; DCHECK(readiobuf_->RemainingCapacity()); } @@ -860,8 +860,8 @@ void TorControl::ReadDone(int rv) { // If we've processed every byte in the input so far, and there's no // more command callbacks queued or asynchronous events registered, // stop. - if (read_start_ == readiobuf_->offset() && cmdq_.empty() && - async_events_.empty()) { + if (read_start_ == base::checked_cast(readiobuf_->offset()) && + cmdq_.empty() && async_events_.empty()) { reading_ = false; readiobuf_.reset(); read_start_ = 0; @@ -1068,7 +1068,7 @@ void TorControl::Error() { } reading_ = false; readiobuf_.reset(); - read_start_ = -1; + read_start_ = 0u; read_cr_ = false; // Clear write state. diff --git a/components/tor/tor_control.h b/components/tor/tor_control.h index 75e0f2259cef..2573a8f45439 100644 --- a/components/tor/tor_control.h +++ b/components/tor/tor_control.h @@ -233,7 +233,7 @@ class TorControl { std::queue> cmdq_; bool reading_; scoped_refptr readiobuf_; - int read_start_; // offset where the current line starts + size_t read_start_; // offset where the current line starts bool read_cr_; // true if we have parsed a CR // Asynchronous command response callback state machine. diff --git a/components/tor/tor_control_unittest.cc b/components/tor/tor_control_unittest.cc index 68ff55c6f858..bb3d4b96782c 100644 --- a/components/tor/tor_control_unittest.cc +++ b/components/tor/tor_control_unittest.cc @@ -136,7 +136,7 @@ TEST(TorControlTest, ReadDone) { "250-SOCKSPORT=9050\r\n250 ORPORT=0\r\n250 OK"; control->reading_ = true; // StartRead() - control->read_start_ = 0; + control->read_start_ = 0u; control->async_events_[tor::TorControlEvent::NETWORK_LIVENESS] = 1; control->readiobuf_ = base::MakeRefCounted(); control->readiobuf_->SetCapacity(sizeof test_str - 1); @@ -147,7 +147,7 @@ TEST(TorControlTest, ReadDone) { // Read so far: 250-SOCKSP control->ReadDone(10); EXPECT_EQ(control->readiobuf_->offset(), 10); - EXPECT_EQ(control->read_start_, 0); + EXPECT_EQ(control->read_start_, 0u); // First CRLF // Read this time: ORT=9050\r\n @@ -156,7 +156,7 @@ TEST(TorControlTest, ReadDone) { EXPECT_CALL(delegate, OnTorRawMid("250", "SOCKSPORT=9050")).Times(1); control->ReadDone(10); EXPECT_EQ(control->readiobuf_->offset(), 20); - EXPECT_EQ(control->read_start_, 20); + EXPECT_EQ(control->read_start_, 20u); // Second CRLF // Read this time: 250 ORPORT=0\r\n2 @@ -165,17 +165,17 @@ TEST(TorControlTest, ReadDone) { EXPECT_CALL(delegate, OnTorRawEnd("250", "ORPORT=0")).Times(1); control->ReadDone(15); EXPECT_EQ(control->readiobuf_->offset(), 35); - EXPECT_EQ(control->read_start_, 34); + EXPECT_EQ(control->read_start_, 34u); // Buffer will be full // Read this time: 50 OK // Read so far: 250 OK control->ReadDone(5); EXPECT_EQ(control->readiobuf_->offset(), 6); - EXPECT_EQ(control->read_start_, 0); - EXPECT_EQ(base::as_string_view( - control->readiobuf_->everything().subspan(0, 6)), - "250 OK"); + EXPECT_EQ(control->read_start_, 0u); + EXPECT_EQ( + base::as_string_view(control->readiobuf_->everything().first<6>()), + "250 OK"); run_loop.Quit(); })); run_loop.Run();