Skip to content

Commit

Permalink
[Spanify] span size/offset to conform to size_t (#26822)
Browse files Browse the repository at this point in the history
There's some tightening around values passed into span, with
`StrictNumeric<size_t>` 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 <[email protected]>
Date:   Tue Nov 26 22:58:16 2024 +0000

    Use StrictNumeric<size_t> 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 brave/brave-browser#42627
  • Loading branch information
cdesouza-chromium authored Dec 3, 2024
1 parent fb8e6af commit d163dde
Show file tree
Hide file tree
Showing 12 changed files with 51 additions and 50 deletions.
3 changes: 2 additions & 1 deletion browser/ui/views/tabs/tab_drag_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ void TabDragController::DetachAndAttachToNewContext(
std::vector<tabs::TabHandle> 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<size_t>(first_tab_index()));
for (const auto& data : drag_data) {
tabs.push_back(tab_strip_model->GetTabHandleAt(
tab_strip_model->GetIndexOfWebContents(data.contents)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::span(data).subspan(1));
::VerifyModel(model, base::span(data).subspan(1u));
}

void RecentTabsSubMenuModelTest::VerifyModel(const ui::MenuModel* model,
Expand Down
4 changes: 2 additions & 2 deletions components/brave_wallet/browser/eth_data_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ std::optional<SquidSwapData> 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);

Expand Down Expand Up @@ -544,7 +544,7 @@ GetTransactionInfoFromData(const std::vector<uint8_t>& data) {
std::vector<std::string>(), 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) {
Expand Down
14 changes: 7 additions & 7 deletions components/brave_wallet/browser/json_rpc_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ class GetAccountInfoHandler : public SolRpcCallHandler {

static std::vector<uint8_t> MakeMintData(int supply) {
std::vector<uint8_t> 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;
}
Expand All @@ -421,10 +421,10 @@ class GetAccountInfoHandler : public SolRpcCallHandler {
std::vector<uint8_t> 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.last(data.size()).copy_from(data);

return result;
}
Expand All @@ -436,15 +436,15 @@ class GetAccountInfoHandler : public SolRpcCallHandler {
std::vector<uint8_t> 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<uint8_t> message;
message.insert(message.end(), sol_record_payload_address.bytes().begin(),
sol_record_payload_address.bytes().end());
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<const uint8_t*>(hex_message.data()),
hex_message.length(), signer_key.data());

Expand Down Expand Up @@ -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);

Expand Down
12 changes: 6 additions & 6 deletions components/brave_wallet/browser/sns_resolver_task.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ base::span<const uint8_t> ExtractSpan(base::span<const uint8_t>& data,
if (data.size() < size) {
return {};
}
auto result = data.subspan(0, size);
auto result = data.first(size);
data = data.subspan(size);
return result;
}
Expand Down Expand Up @@ -239,8 +239,8 @@ std::optional<SolanaAddress> 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<kSolanaPubkeySize>());
if (!sol_record_payload_address) {
return std::nullopt;
}
Expand Down Expand Up @@ -588,15 +588,15 @@ std::optional<NameRegistryState> 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<kSolanaPubkeySize>());
data_span = data_span.subspan(kSolanaPubkeySize);

result->owner =
*SolanaAddress::FromBytes(data_span.subspan(0, kSolanaPubkeySize));
*SolanaAddress::FromBytes(data_span.first<kSolanaPubkeySize>());
data_span = data_span.subspan(kSolanaPubkeySize);

result->data_class =
*SolanaAddress::FromBytes(data_span.subspan(0, kSolanaPubkeySize));
*SolanaAddress::FromBytes(data_span.first<kSolanaPubkeySize>());
data_span = data_span.subspan(kSolanaPubkeySize);

result->data.assign(data_span.begin(), data_span.end());
Expand Down
10 changes: 5 additions & 5 deletions components/brave_wallet/common/brave_wallet_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 8 additions & 8 deletions components/brave_wallet/common/eth_abi_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -102,7 +103,7 @@ std::optional<Span32> 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
Expand All @@ -114,7 +115,7 @@ std::pair<Span, Span> ExtractFunctionSelectorAndArgsFromCall(Span data) {
if ((data.size() - 4) % kRowLength) {
return {};
}
return {data.subspan(0, 4), data.subspan(4)};
return data.split_at<4>();
}

std::pair<std::optional<size_t>, Span> ExtractArrayInfo(Span data) {
Expand Down Expand Up @@ -167,8 +168,7 @@ std::optional<std::vector<uint8_t>> 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<uint8_t>{bytes_result.begin(), bytes_result.end()};
return base::ToVector(padded_bytes_data.first(*bytes_len));
}

std::optional<std::string> ExtractString(Span string_encoded) {
Expand All @@ -194,8 +194,8 @@ std::optional<std::string> 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<std::vector<std::string>> ExtractStringArray(Span string_array) {
Expand Down Expand Up @@ -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<uint8_t>{head->begin(), head->begin() + fixed_size};
return base::ToVector(head->first(fixed_size));
}

// NOLINTNEXTLINE(runtime/references)
Expand Down
10 changes: 5 additions & 5 deletions components/brave_wallet/common/zcash_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,16 @@ std::optional<uint64_t> ReadCompactSize(base::span<const uint8_t>& 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;
}
Expand Down Expand Up @@ -97,7 +97,7 @@ std::optional<std::vector<ParsedAddress>> 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_t>(*size));
}
return result;
}
Expand Down
2 changes: 1 addition & 1 deletion components/ipfs/ipfs_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ base::span<const uint8_t> DecodeVarInt(base::span<const uint8_t> from,
shift += 7;
} while (*it++ & 0x80);
*into = static_cast<int64_t>(ret);
return from.subspan(it - from.begin());
return from.subspan(static_cast<size_t>(std::distance(from.begin(), it)));
}

// Extracts cid and path from ipfs URLs like:
Expand Down
10 changes: 5 additions & 5 deletions components/tor/tor_control.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ TorControl::TorControl(base::WeakPtr<TorControl::Delegate> 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_);
Expand Down Expand Up @@ -730,7 +730,7 @@ void TorControl::StartRead() {
DCHECK(!cmdq_.empty() || !async_events_.empty());
readiobuf_ = base::MakeRefCounted<net::GrowableIOBuffer>();
readiobuf_->SetCapacity(kTorBufferSize);
read_start_ = 0;
read_start_ = 0u;
DCHECK(readiobuf_->RemainingCapacity());
}

Expand Down Expand Up @@ -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<size_t>(readiobuf_->offset()) &&
cmdq_.empty() && async_events_.empty()) {
reading_ = false;
readiobuf_.reset();
read_start_ = 0;
Expand Down Expand Up @@ -1068,7 +1068,7 @@ void TorControl::Error() {
}
reading_ = false;
readiobuf_.reset();
read_start_ = -1;
read_start_ = 0u;
read_cr_ = false;

// Clear write state.
Expand Down
2 changes: 1 addition & 1 deletion components/tor/tor_control.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ class TorControl {
std::queue<std::pair<PerLineCallback, CmdCallback>> cmdq_;
bool reading_;
scoped_refptr<net::GrowableIOBuffer> 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.
Expand Down
16 changes: 8 additions & 8 deletions components/tor/tor_control_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<net::GrowableIOBuffer>();
control->readiobuf_->SetCapacity(sizeof test_str - 1);
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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();
Expand Down

0 comments on commit d163dde

Please sign in to comment.