forge-std icon indicating copy to clipboard operation
forge-std copied to clipboard

bug: stdJson readBytes cannot handle 20-byte values

Open dcposch opened this issue 1 year ago • 3 comments

Summary

stdJson appears to be broken in latest forge-std. Reading 20-byte values with readBytes(json, path) returns incorrect results or reverts.

Versions

forge-std: v1.2.0

forge --version
forge 0.2.0 (26a7559 2024-07-31T00:19:23.655582000Z)

Minimal reproduction

pragma solidity ^0.8.21;

import {Test} from "forge-std/Test.sol";
import {stdJson} from "forge-std/StdJson.sol";

using stdJson for string;

contract JsonTest is Test {
    function testJson() public {
        string memory data = '{"data":"0x1234"}';
        bytes memory hello = data.readBytes(".data");
        assertEq(hello, hex"1234");

        // 19 bytes = works
        data = '{"data":"0x00000000000000000000000000000000000000"}';
        hello = data.readBytes(".data");
        assertEq(hello, hex"00000000000000000000000000000000000000");

        // 21 bytes = works
        data = '{"data":"0x000000000000000000000000000000000000000000"}';
        hello = data.readBytes(".data");
        assertEq(hello, hex"000000000000000000000000000000000000000000");

        // 20 bytes = returns without error, WRONG DATA
        data = '{"data":"0x0000000000000000000000000000000000000000"}';
        hello = data.readBytes(".data");
        assertEq(hello, hex"");

        // 20 bytes = REVERTS
        data = '{"data":"0x4bf5122f344554c53bde2ebb8cd2b7e3d1600ad6"}';
        data.readBytes(".data");
    }
}

dcposch avatar Jul 31 '24 06:07 dcposch

Do you know if this was always the case, or was there a regression in foundry or forge-std? The reverting values are actually 20 bytes long (40 hex chars), so my guess is that it's getting parsed as an address

cc @mattsse to assign someone to look into this :)

mds1 avatar Jul 31 '24 15:07 mds1

Regression. The test suite where we ran into this issue passed on an earlier version of forge-std

20 bytes

Yeah exactly, typo; fixed.

& yes: I think something internal is automatically parsing 20-byte hex as an address. If true, it's very similar to the the last bug we found in Foundry JSON handling: https://github.com/foundry-rs/foundry/issues/5808

Maybe a root cause fix: no automatic type inference. Users of stdJson should always explicitly specify types + formats.

dcposch avatar Jul 31 '24 21:07 dcposch

Looks like this is only happening on older forge-std versions

forge-std 1.2.0 does abi.decode(vm.parseJson(json, key), (bytes)); instead of vm.parseJsonBytes(json, key);, so automatic coercion to address might cause errors during abi decoding. Not sure why it may have regressed, I think parseJson always did that coercion?

klkvr avatar Aug 02 '24 15:08 klkvr