Skip to content

Commit c62f26d

Browse files
AmxxAmxx
committed
Fix bug in Bytes.lastIndexOf when array is empty and position is not 2²⁵⁶-1
1 parent 32397f2 commit c62f26d

File tree

3 files changed

+65
-16
lines changed

3 files changed

+65
-16
lines changed

‎contracts/utils/Bytes.sol

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,12 @@ library Bytes {
5858
function lastIndexOf(bytes memory buffer, bytes1 s, uint256 pos) internal pure returns (uint256) {
5959
unchecked {
6060
uint256 length = buffer.length;
61-
// NOTE here we cannot do `i = Math.min(pos + 1, length)` because `pos + 1` could overflow
62-
for (uint256 i = Math.min(pos, length - 1) + 1; i > 0; --i) {
63-
if (bytes1(_unsafeReadBytesOffset(buffer, i - 1)) == s) {
64-
return i - 1;
61+
if (length > 0) {
62+
// NOTE here we cannot do `i = Math.min(pos + 1, length)` because `pos + 1` could overflow
63+
for (uint256 i = Math.min(pos, length - 1) + 1; i > 0; --i) {
64+
if (bytes1(_unsafeReadBytesOffset(buffer, i - 1)) == s) {
65+
return i - 1;
66+
}
6567
}
6668
}
6769
return type(uint256).max;

‎test/utils/Bytes.t.sol

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,37 @@ import {Bytes} from "@openzeppelin/contracts/utils/Bytes.sol";
99
contract BytesTest is Test {
1010
using Bytes for bytes;
1111

12+
// INDEX OF
13+
function testIndexOf(bytes memory buffer, bytes1 s) public pure {
14+
testIndexOf(buffer, s, 0);
15+
}
16+
17+
function testIndexOf(bytes memory buffer, bytes1 s, uint256 pos) public pure {
18+
uint256 result = Bytes.indexOf(buffer, s, pos);
19+
20+
// The search value should not me present between `pos` (included) and `result` excluded.
21+
// Do not search after the end of the buffer
22+
for (uint256 i = pos; i < Math.min(result, buffer.length); ++i) assertNotEq(buffer[i], s);
23+
if (result != type(uint256).max) assertEq(buffer[result], s);
24+
}
25+
26+
function testLastIndexOf(bytes memory buffer, bytes1 s) public pure {
27+
testLastIndexOf(buffer, s, 0);
28+
}
29+
30+
function testLastIndexOf(bytes memory buffer, bytes1 s, uint256 pos) public pure {
31+
uint256 result = Bytes.lastIndexOf(buffer, s, pos);
32+
33+
// Case found: the search value should not be present between `result` (excluded) and `pos` (included)
34+
// Case not found: the search value should not be present anywhere before `pos` (included)
35+
unchecked {
36+
// using unchecked gives us `result + 1 == 0` in the "not found" case
37+
for (uint256 i = result + 1; i < Math.min(pos + 1, buffer.length); ++i) assertNotEq(buffer[i], s);
38+
}
39+
if (result != type(uint256).max) assertEq(buffer[result], s);
40+
}
41+
42+
// SLICES
1243
function testSliceWithStartOnly(bytes memory buffer, uint256 start) public pure {
1344
bytes memory originalBuffer = bytes.concat(buffer);
1445
bytes memory result = buffer.slice(start);

‎test/utils/Bytes.test.js

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,39 +28,55 @@ describe('Bytes', function () {
2828

2929
describe('indexOf', function () {
3030
it('first', async function () {
31-
expect(await this.mock.$indexOf(lorem, ethers.toBeHex(present))).to.equal(lorem.indexOf(present));
31+
await expect(this.mock.$indexOf(lorem, ethers.toBeHex(present))).to.eventually.equal(lorem.indexOf(present));
3232
});
3333

3434
it('from index', async function () {
3535
for (const start in Array(lorem.length + 10).fill()) {
3636
const index = lorem.indexOf(present, start);
3737
const result = index === -1 ? ethers.MaxUint256 : index;
38-
expect(await this.mock.$indexOf(lorem, ethers.toBeHex(present), ethers.Typed.uint256(start))).to.equal(result);
38+
await expect(
39+
this.mock.$indexOf(lorem, ethers.toBeHex(present), ethers.Typed.uint256(start)),
40+
).to.eventually.equal(result);
3941
}
4042
});
4143

4244
it('absent', async function () {
43-
expect(await this.mock.$indexOf(lorem, ethers.toBeHex(absent))).to.equal(ethers.MaxUint256);
45+
await expect(this.mock.$indexOf(lorem, ethers.toBeHex(absent))).to.eventually.equal(ethers.MaxUint256);
46+
});
47+
48+
it('empty buffer', async function () {
49+
await expect(this.mock.$indexOf('0x', '0x00')).to.eventually.equal(ethers.MaxUint256);
50+
await expect(this.mock.$indexOf('0x', '0x00', ethers.Typed.uint256(17))).to.eventually.equal(ethers.MaxUint256);
4451
});
4552
});
4653

4754
describe('lastIndexOf', function () {
4855
it('first', async function () {
49-
expect(await this.mock.$lastIndexOf(lorem, ethers.toBeHex(present))).to.equal(lorem.lastIndexOf(present));
56+
await expect(this.mock.$lastIndexOf(lorem, ethers.toBeHex(present))).to.eventually.equal(
57+
lorem.lastIndexOf(present),
58+
);
5059
});
5160

5261
it('from index', async function () {
5362
for (const start in Array(lorem.length + 10).fill()) {
5463
const index = lorem.lastIndexOf(present, start);
5564
const result = index === -1 ? ethers.MaxUint256 : index;
56-
expect(await this.mock.$lastIndexOf(lorem, ethers.toBeHex(present), ethers.Typed.uint256(start))).to.equal(
57-
result,
58-
);
65+
await expect(
66+
this.mock.$lastIndexOf(lorem, ethers.toBeHex(present), ethers.Typed.uint256(start)),
67+
).to.eventually.equal(result);
5968
}
6069
});
6170

6271
it('absent', async function () {
63-
expect(await this.mock.$lastIndexOf(lorem, ethers.toBeHex(absent))).to.equal(ethers.MaxUint256);
72+
await expect(this.mock.$lastIndexOf(lorem, ethers.toBeHex(absent))).to.eventually.equal(ethers.MaxUint256);
73+
});
74+
75+
it('empty buffer', async function () {
76+
await expect(this.mock.$lastIndexOf('0x', '0x00')).to.eventually.equal(ethers.MaxUint256);
77+
await expect(this.mock.$lastIndexOf('0x', '0x00', ethers.Typed.uint256(17))).to.eventually.equal(
78+
ethers.MaxUint256,
79+
);
6480
});
6581
});
6682

@@ -73,8 +89,8 @@ describe('Bytes', function () {
7389
})) {
7490
it(descr, async function () {
7591
const result = ethers.hexlify(lorem.slice(start));
76-
expect(await this.mock.$slice(lorem, start)).to.equal(result);
77-
expect(await this.mock.$splice(lorem, start)).to.equal(result);
92+
await expect(this.mock.$slice(lorem, start)).to.eventually.equal(result);
93+
await expect(this.mock.$splice(lorem, start)).to.eventually.equal(result);
7894
});
7995
}
8096
});
@@ -89,8 +105,8 @@ describe('Bytes', function () {
89105
})) {
90106
it(descr, async function () {
91107
const result = ethers.hexlify(lorem.slice(start, end));
92-
expect(await this.mock.$slice(lorem, start, ethers.Typed.uint256(end))).to.equal(result);
93-
expect(await this.mock.$splice(lorem, start, ethers.Typed.uint256(end))).to.equal(result);
108+
await expect(this.mock.$slice(lorem, start, ethers.Typed.uint256(end))).to.eventually.equal(result);
109+
await expect(this.mock.$splice(lorem, start, ethers.Typed.uint256(end))).to.eventually.equal(result);
94110
});
95111
}
96112
});

0 commit comments

Comments
 (0)