Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: rename alphanumeric function #949

Merged
merged 2 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion precompiles/Precompiles.sol

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions src/SablierV2NFTDescriptor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ contract SablierV2NFTDescriptor is ISablierV2NFTDescriptor {
}

/// @notice Checks whether the provided string contains only alphanumeric characters and spaces.
/// @dev Note that this returns true for empty strings, but it is not a security concern.
function isAlphanumeric(string memory str) internal pure returns (bool) {
/// @dev Note that this returns true for empty strings.
function isAlphanumericWithSpaces(string memory str) internal pure returns (bool) {
// Convert the string to bytes to iterate over its characters.
bytes memory b = bytes(str);

Expand Down Expand Up @@ -368,8 +368,8 @@ contract SablierV2NFTDescriptor is ISablierV2NFTDescriptor {
if (bytes(symbol).length > 30) {
return "Long Symbol";
} else {
if (!isAlphanumeric(symbol)) {
return "Non-Alphanumeric Symbol";
if (!isAlphanumericWithSpaces(symbol)) {
return "Unsupported Symbol";
}
return symbol;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.22 <0.9.0;

import { NFTDescriptor_Integration_Shared_Test } from "../../../shared/nft-descriptor/NFTDescriptor.t.sol";

contract IsAlphanumericWithSpaces_Integration_Concrete_Test is NFTDescriptor_Integration_Shared_Test {
function test_IsAlphanumericWithSpaces_EmptyString() external view {
string memory symbol = "";
bool result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertTrue(result, "isAlphanumericWithSpaces");
}

modifier whenNotEmptyString() {
_;
}

function test_IsAlphanumericWithSpaces_ContainsUnsupportedCharacters() external view whenNotEmptyString {
string memory symbol = "<foo/>";
bool result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo/";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo\\";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo%";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo&";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo(";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo)";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo\"";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo'";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo`";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo;";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");

symbol = "foo%20"; // URL-encoded empty space
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertFalse(result, "isAlphanumericWithSpaces");
}

modifier whenOnlySupportedCharacters() {
_;
}

function test_IsAlphanumericWithSpaces() external view whenNotEmptyString whenOnlySupportedCharacters {
string memory symbol = "foo";
bool result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertTrue(result, "isAlphanumericWithSpaces");

symbol = "Foo";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertTrue(result, "isAlphanumericWithSpaces");

symbol = "Foo ";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertTrue(result, "isAlphanumericWithSpaces");

symbol = "Foo Bar";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertTrue(result, "isAlphanumericWithSpaces");

symbol = " ";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertTrue(result, "isAlphanumericWithSpaces");

symbol = "foo01234";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertTrue(result, "isAlphanumericWithSpaces");

symbol = "123456789";
result = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertTrue(result, "isAlphanumericWithSpaces");
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
isAlphanumeric.t.sol
isAlphanumericWithSpaces.t.sol
├── when the symbol is an empty string
│ └── it should return true
└── when the symbol is not an empty string
├── given the symbol does not contain only alphanumeric characters
├── given the symbol contain unsupported characters
│ └── it should return false
└── given the symbol contains only alphanumeric characters
└── given the symbol contains only supported characters
└── it should return true

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ contract SafeAssetSymbol_Integration_Concrete_Test is NFTDescriptor_Integration_
function test_SafeAssetSymbol_NonAlphanumeric() external whenERC20Contract givenSymbolString givenSymbolNotLong {
ERC20Mock asset = new ERC20Mock({ name: "Token", symbol: "<svg/onload=alert(\"xss\")>" });
string memory actualSymbol = nftDescriptorMock.safeAssetSymbol_(address(asset));
string memory expectedSymbol = "Non-Alphanumeric Symbol";
string memory expectedSymbol = "Unsupported Symbol";
assertEq(actualSymbol, expectedSymbol, "symbol");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity >=0.8.22 <0.9.0;

import { NFTDescriptor_Integration_Shared_Test } from "../../shared/nft-descriptor/NFTDescriptor.t.sol";

contract IsAlphanumeric_Integration_Fuzz_Test is NFTDescriptor_Integration_Shared_Test {
contract IsAlphanumericWithSpaces_Integration_Fuzz_Test is NFTDescriptor_Integration_Shared_Test {
bytes1 internal constant SPACE = 0x20; // ASCII 32
bytes1 internal constant ZERO = 0x30; // ASCII 48
bytes1 internal constant NINE = 0x39; // ASCII 57
Expand All @@ -21,22 +21,22 @@ contract IsAlphanumeric_Integration_Fuzz_Test is NFTDescriptor_Integration_Share
/// - String with only alphanumerical characters
/// - String with only non-alphanumerical characters
/// - String with both alphanumerical and non-alphanumerical characters
function testFuzz_IsAlphanumeric(string memory symbol) external view whenNotEmptyString {
function testFuzz_IsAlphanumericWithSpaces(string memory symbol) external view whenNotEmptyString {
bytes memory b = bytes(symbol);
uint256 length = b.length;
bool expectedResult = true;
for (uint256 i = 0; i < length; ++i) {
bytes1 char = b[i];
if (!isAlphanumericChar(char)) {
if (!isAlphanumericOrSpaceChar(char)) {
expectedResult = false;
break;
}
}
bool actualResult = nftDescriptorMock.isAlphanumeric_(symbol);
assertEq(actualResult, expectedResult, "isAlphanumeric");
bool actualResult = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
assertEq(actualResult, expectedResult, "isAlphanumericWithSpaces");
}

function isAlphanumericChar(bytes1 char) internal pure returns (bool) {
function isAlphanumericOrSpaceChar(bytes1 char) internal pure returns (bool) {
bool isSpace = char == SPACE;
bool isDigit = char >= ZERO && char <= NINE;
bool isUppercaseLetter = char >= A && char <= Z;
Expand Down
4 changes: 2 additions & 2 deletions test/mocks/NFTDescriptorMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ contract NFTDescriptorMock is SablierV2NFTDescriptor {
return SVGElements.hourglass(status);
}

function isAlphanumeric_(string memory symbol) external pure returns (bool) {
return isAlphanumeric(symbol);
function isAlphanumericWithSpaces_(string memory symbol) external pure returns (bool) {
return isAlphanumericWithSpaces(symbol);
}

function mapSymbol_(IERC721Metadata nft) external view returns (string memory) {
Expand Down