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

Add strict validation mode for AR resource identifier schemes #2453

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

nvmkuruc
Copy link
Collaborator

@nvmkuruc nvmkuruc commented May 31, 2023

Description of Change(s)

The current implementation of AR resource identifiers (URI/IRI) allows any string to be registered as the scheme component. The URI/IRI specification restricts the character set of the scheme component to ASCII alphanumeric characters plus +, ., and -. Consider handling of a scheme which contains : as an example of where an unrestricted character set for scheme can go awry. This change adds validation, warning the user when a scheme is not valid under the specification. By default, the scheme is ignored, but users can set PXR_AR_DISABLE_STRICT_SCHEME_VALIDATION to be more permissive.

Validation could be implemented as a regular expression match, but was done with explicit checks to produce clearer error messages for users. A note is left to output the invalid characters once #2120 has been accepted.

The test_output scheme currently used in some of the testing is an example of an invalid scheme per the URI/IRI specifications. This scheme has been added to a list of invalid test cases and been replaced with test-output.

Fixes Issue(s)

  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

@sunyab
Copy link
Contributor

sunyab commented Jun 2, 2023

Filed as internal issue #USD-8372

@pixar-oss pixar-oss merged commit ff00bf1 into PixarAnimationStudios:dev Oct 2, 2023
5 checks passed
@nvmkuruc nvmkuruc deleted the aririscheme branch December 29, 2023 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants