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

Fix another problem in parseGltfCopyright #1109

Merged
merged 2 commits into from
Feb 12, 2025
Merged

Fix another problem in parseGltfCopyright #1109

merged 2 commits into from
Feb 12, 2025

Conversation

kring
Copy link
Member

@kring kring commented Feb 12, 2025

Sadly, my fix for parseGltfCopyright in was subtly broken.

As reported here:
CesiumGS/cesium-unity#551 (comment)

Explanation copied from the comment there:

There's a line of code like this:

s.find_first_not_of(" \t", 0, end + 1)

Which I thought would find the first character that isn't space or tab starting at position 0 in s and continuing for end+1 characters. But no! That last parameter is the length of the string in the first parameter, not the number of characters to search. That is... really unexpected. And amazingly, our rather extensive tests of this function passed on all platforms, in both debug and release even with this mistake. But it seems that if the contents of memory after that " \t" string is juuuust right, which it happens to be in your model and in Unity (but only in a Release build), then this function will do the wrong thing.

This has already gone out in a hotfix for both Unreal (v2.13.3) and Unity (v1.15.3).

@j9liu
Copy link
Contributor

j9liu commented Feb 12, 2025

Thanks @kring for dealing with this string parsing madness 🥲

@j9liu j9liu merged commit e1f53f7 into main Feb 12, 2025
22 checks passed
@j9liu j9liu deleted the patch-v0.44 branch February 12, 2025 14:51
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.

2 participants