Skip to content

Fix UTF-8 decoding for glTF URIs - #3070

Open
adrian-andrzjewski wants to merge 1 commit into
jMonkeyEngine:masterfrom
adrian-andrzjewski:codex/issue-2124-gltf-unicode
Open

adrian-andrzjewski wants to merge 1 commit into
jMonkeyEngine:masterfrom
adrian-andrzjewski:codex/issue-2124-gltf-unicode

Conversation

@adrian-andrzjewski

Copy link
Copy Markdown

Fixes #2124

Summary

GsonParser wrapped JSON input in an InputStreamReader without specifying a charset. On Windows code pages, that decodes UTF-8 glTF files with the wrong charset and corrupts literal non-ASCII resource URIs before GltfLoader resolves them.

This change decodes JSON streams as UTF-8 and adds an integration regression test that loads a glTF model with Unicode in its folder name and external buffer URI.

Validation

  • Reproduced the regression before the fix with -Dfile.encoding=windows-1252; loading failed with AssetNotFoundException for the corrupted buffer URI.
  • JAVA_TOOL_OPTIONS=-Dfile.encoding=windows-1252 .\gradlew.bat :jme3-plugins:test --tests com.jme3.scene.plugins.gltf.GltfLoaderTest.loadsExternalBufferWithUnicodeUri --no-daemon — passed.
  • .\gradlew.bat :jme3-plugins:check :jme3-plugins-json-gson:check --no-daemon — passed.

@adrian-andrzjewski

Copy link
Copy Markdown
Author

@riccardobl Could you review PR #3070? It makes Gson input decoding explicit UTF-8 and adds a glTF regression test for the Windows code-page failure.

@jaime-jmebot jaime-jmebot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Adrian, thanks for putting together this fix and a focused regression test. The change addresses the root cause cleanly: glTF JSON is UTF-8, so explicitly selecting StandardCharsets.UTF_8 when constructing the reader prevents the platform default charset from corrupting literal URI characters before the loader can resolve them.

I also checked the integration test path. It writes both the model and external buffer under Unicode-containing names, registers the temporary directory with the file locator, and verifies the loaded geometry has its position data. That exercises the actual asset-loading flow rather than only testing parser output, which makes this a useful regression guard for the reported failure.

I didn’t find any issues in the changes. The implementation is appropriately scoped, and the reported checks cover the affected modules. Looks good to me—thanks!

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.

jme3-plugins fails to load UnicodeTest

2 participants