diff --git a/contracts/utils/ShortStrings.sol b/contracts/utils/ShortStrings.sol index 9f253df82..2f891aaa3 100644 --- a/contracts/utils/ShortStrings.sol +++ b/contracts/utils/ShortStrings.sol @@ -32,7 +32,10 @@ type ShortString is bytes32; * ``` */ library ShortStrings { + bytes32 private constant _FALLBACK_SENTINEL = 0x00000000000000000000000000000000000000000000000000000000000000FF; + error StringTooLong(string str); + error InvalidShortString(); /** * @dev Encode a string of at most 31 chars into a `ShortString`. @@ -66,7 +69,11 @@ library ShortStrings { * @dev Return the length of a `ShortString`. */ function length(ShortString sstr) internal pure returns (uint256) { - return uint256(ShortString.unwrap(sstr)) & 0xFF; + uint256 result = uint256(ShortString.unwrap(sstr)) & 0xFF; + if (result > 31) { + revert InvalidShortString(); + } + return result; } /** @@ -77,7 +84,7 @@ library ShortStrings { return toShortString(value); } else { StorageSlot.getStringSlot(store).value = value; - return ShortString.wrap(0); + return ShortString.wrap(_FALLBACK_SENTINEL); } } @@ -85,7 +92,7 @@ library ShortStrings { * @dev Decode a string that was encoded to `ShortString` or written to storage using {setWithFallback}. */ function toStringWithFallback(ShortString value, string storage store) internal pure returns (string memory) { - if (length(value) > 0) { + if (ShortString.unwrap(value) != _FALLBACK_SENTINEL) { return toString(value); } else { return store; diff --git a/test/utils/ShortStrings.test.js b/test/utils/ShortStrings.test.js index f41084a7e..27d52a5d7 100644 --- a/test/utils/ShortStrings.test.js +++ b/test/utils/ShortStrings.test.js @@ -3,9 +3,12 @@ const { expectRevertCustomError } = require('../helpers/customError'); const ShortStrings = artifacts.require('$ShortStrings'); +function length(sstr) { + return parseInt(sstr.slice(64), 16); +} + function decode(sstr) { - const length = parseInt(sstr.slice(64), 16); - return web3.utils.toUtf8(sstr).slice(0, length); + return web3.utils.toUtf8(sstr).slice(0, length(sstr)); } contract('ShortStrings', function () { @@ -34,7 +37,12 @@ contract('ShortStrings', function () { const { logs } = await this.mock.$toShortStringWithFallback(str, 0); const { ret0 } = logs.find(({ event }) => event == 'return$toShortStringWithFallback').args; - expect(await this.mock.$toString(ret0)).to.be.equal(str.length < 32 ? str : ''); + const promise = this.mock.$toString(ret0); + if (str.length < 32) { + expect(await promise).to.be.equal(str); + } else { + await expectRevertCustomError(promise, 'InvalidShortString()'); + } const recovered = await this.mock.$toStringWithFallback(ret0, 0); expect(recovered).to.be.equal(str);