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.

Sign up for GitHub

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

Jump to bottom

Inconsistent results between fromString and isValid methods. #531

Open
rogervila opened this issue Jan 16, 2024 · 2 comments · May be fixed by #533
Open

Inconsistent results between fromString and isValid methods. #531

rogervila opened this issue Jan 16, 2024 · 2 comments · May be fixed by #533
Labels
enhancement

Comments

@rogervila
Copy link

Description

When creating a UUID instance from a string without dashes (ie: 00000000000000000000000000000000) it works, but when receiving the same string to validate if it's a valid UUID it fails, which is inconsistent.

The library should not be able to create a UUID for a string that is not valid.

Steps to reproduce

  1. Create a UUID from a 00000000000000000000000000000000 string. It returns a UUID instance
  2. validate 00000000000000000000000000000000 using the isValid function. It returns false.

Expected behavior

The library should not be able to create a UUID for a string that is not valid.

Screenshots or output

Input and output from Laravel tinker:

> \Ramsey\Uuid\Uuid::fromString('00000000000000000000000000000000')
= Ramsey\Uuid\Rfc4122\NilUuid {#5021
    uuid: "00000000-0000-0000-0000-000000000000",
  }

> \Ramsey\Uuid\Uuid::isValid('00000000000000000000000000000000')
= false

Environment details

  • version of this package: 4.7.5
  • PHP version: 8.2.14
  • OS: macOS 13.6.1

Additional context
@rogervila rogervila added the bug label Jan 16, 2024
@ramsey
Copy link
Owner

ramsey commented Jan 17, 2024

The validator expects the UUID to be formatted as a UUID string (including dashes):

uuid/src/Validator/GenericValidator.php

Line 32 in 758682d

private const VALID_PATTERN = '\A[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}\z';

While the fromString() method (by default) uses StringCodec::getBytes() to parse the string into bytes, and while it still uses Uuid::isValid() to check validity, it ignores whether the original string contains dashes (in fact, it adds the dashes to the original string before checking validity):

uuid/src/Codec/StringCodec.php

Lines 107 to 130 in 758682d

protected function getBytes(string $encodedUuid): string
{
$parsedUuid = str_replace(
['urn:', 'uuid:', 'URN:', 'UUID:', '{', '}', '-'],
'',
$encodedUuid
);
$components = [
substr($parsedUuid, 0, 8),
substr($parsedUuid, 8, 4),
substr($parsedUuid, 12, 4),
substr($parsedUuid, 16, 4),
substr($parsedUuid, 20),
];
if (!Uuid::isValid(implode('-', $components))) {
throw new InvalidUuidStringException(
'Invalid UUID string: ' . $encodedUuid
);
}
return (string) hex2bin($parsedUuid);
}

I'm not sure this is a bug, since changing the validator could cause applications to break, if they assume the strings must have dashes in them, and if we require dashes in the value passed to fromString(), that will break applications that don't use dashes when storing to the database, etc.

@rogervila
Copy link
Author

Thank you for your explanation @ramsey

Therefore it would be nice to have a fromStrictString method that validates the string before doing any conversion on it.
Would you accept a PR for that?

@rogervila rogervila linked a pull request Jan 26, 2024 that will close this issue
Open
7 tasks
@ramsey ramsey added enhancement and removed bug labels Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fromStrictString method rogervila/ramsey-uuid
2 participants
@ramsey @rogervila