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

[sdf, usd] Fix crash when URL-encoded characters are used to generate session layer identifiers #2022

Merged

Conversation

asluk
Copy link
Collaborator

@asluk asluk commented Sep 4, 2022

[sdf, usd] Fix crash when URL-encoded characters are used to generate session layer identifiers

When opening a stage whose root layer has URL-encoded characters, sequences such as %20 for spaces could get misinterpreted as format strings when generating session layer identifiers on the fly via TfStringPrintf. This change uses stringstreams to ensure that URL-encoded characters never get misinterpreted on this codepath, which was previously leading to crashes such as the one in the testUsdStage case added here.

Thanks to Seema Kulkarni and Edward Slavin for earlier work on this PR.

  • I have submitted a signed Contributor License Agreement

@asluk
Copy link
Collaborator Author

asluk commented Sep 4, 2022

Looks like there are some newly failing tests after I rebased to latest dev; will try to look into those over the next couple weeks.

@asluk asluk force-pushed the specialchars_squashed branch from 6ce8a3c to b7d54c7 Compare September 4, 2022 13:24
@tallytalwar
Copy link
Contributor

Filed as internal issue #USD-7612

@asluk asluk force-pushed the specialchars_squashed branch from b7d54c7 to 92aea95 Compare September 29, 2022 10:34
@meshula
Copy link
Member

meshula commented Sep 29, 2022

Looks good, thanks!

@meshula
Copy link
Member

meshula commented Sep 30, 2022

Hi @asluk ! I haven't debugged yet to see what's going wrong, but I wanted to give you a heads up that I haven't had a successful test run ~

====================================================================== 
ERROR: test_URLEncodedIdentifiers (__main__.TestUsdStage)
 ---------------------------------------------------------------------- 
Traceback (most recent call last): 

File "/scratch/build_local/dev/src/inst/fedora-gcc64-opt/pxr/tests/testUsdStage", 
line 33, in test_URLEncodedIdentifiers
  stage = Usd.Stage.Open("Libeccio%20LowFBX.usda") 
pixar.Tf.ErrorException:  Error in 'UsdStage::Open' at line 879 in file pxr/usd/usd/stage.cpp : 
   'Failed to open layer @Libeccio%20LowFBX.usda@'

@asluk
Copy link
Collaborator Author

asluk commented Oct 1, 2022

Hi @asluk ! I haven't debugged yet to see what's going wrong, but I wanted to give you a heads up that I haven't had a successful test run ~

====================================================================== 
ERROR: test_URLEncodedIdentifiers (__main__.TestUsdStage)
 ---------------------------------------------------------------------- 
Traceback (most recent call last): 

File "/scratch/build_local/dev/src/inst/fedora-gcc64-opt/pxr/tests/testUsdStage", 
line 33, in test_URLEncodedIdentifiers
  stage = Usd.Stage.Open("Libeccio%20LowFBX.usda") 
pixar.Tf.ErrorException:  Error in 'UsdStage::Open' at line 879 in file pxr/usd/usd/stage.cpp : 
   'Failed to open layer @Libeccio%20LowFBX.usda@'

Thanks @meshula ... it's passing for me on Windows; will try it on Ubuntu via WSL2 next week (are the Azure pipelines running tests? They are all passing, but I forget if they are only compiling). 🙏

@tallytalwar
Copy link
Contributor

@asluk yeah azure pipeline doesn't run tests at present. We want to get to it but I think timeouts are a concern there! https://github.com/PixarAnimationStudios/USD/blob/release/azure-pipelines.yml

@asluk asluk force-pushed the specialchars_squashed branch from 92aea95 to 87dd545 Compare October 2, 2022 08:53
@asluk
Copy link
Collaborator Author

asluk commented Oct 2, 2022

Hi @asluk ! I haven't debugged yet to see what's going wrong, but I wanted to give you a heads up that I haven't had a successful test run ~

====================================================================== 
ERROR: test_URLEncodedIdentifiers (__main__.TestUsdStage)
 ---------------------------------------------------------------------- 
Traceback (most recent call last): 

File "/scratch/build_local/dev/src/inst/fedora-gcc64-opt/pxr/tests/testUsdStage", 
line 33, in test_URLEncodedIdentifiers
  stage = Usd.Stage.Open("Libeccio%20LowFBX.usda") 
pixar.Tf.ErrorException:  Error in 'UsdStage::Open' at line 879 in file pxr/usd/usd/stage.cpp : 
   'Failed to open layer @Libeccio%20LowFBX.usda@'

Thanks @meshula ... it's passing for me on Windows; will try it on Ubuntu via WSL2 next week (are the Azure pipelines running tests? They are all passing, but I forget if they are only compiling). 🙏

Thanks @meshula and @tallytalwar -- the test is passing for me on Windows and on Ubuntu via WSL2; not sure what might be happening differently on Fedora :(

@pixar-oss pixar-oss merged commit abb4c1c into PixarAnimationStudios:dev Nov 1, 2022
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.

4 participants