glTF importer tries to use base64 text as file path #33796
Comments
Welp... Here's a minimal reproduction project which doesn't weigh 850 MB ;) (Includes the two assets referenced in the OP.) |
akien-mga
added a commit
to akien-mga/godot
that referenced
this issue
Oct 2, 2020
…* MIME types See KhronosGroup/glTF#944 for context on the application/gltf-buffer MIME type. The glTF 2.0 spec supports `image/jpeg` and `image/png` which can also be base64-encoded in buffer URIs. Fixes godotengine#33796.
|
For the reference, if you try to import https://github.com/KhronosGroup/glTF-Sample-Models after the fix, you'll now get a crash \o/ but it's unrelated to this glTF issue. There are actually some DAE files in the repo which trigger a crash in the Collada importer. I'll open an issue for that. |
|
I merged a bit too fast, after removing the Collada files there are still some glTF files which can fail. I'll make a follow up PR. |
|
Alright, #42504 fixes the base64-related issues. There's issues more among the ~250 glTF files of that Khronos repo, notably:
I'll open separate issues for those (+ the Collada crash). That's a crazily huge repo but it's a good testing resource for the glTF importer :) |
Note: This repo also contains glTF 1.0 scenes that we don't support, so they raise errors too and fail importing. I'll see if we can gracefully recognize such files and ignore them. |
akien-mga
changed the title
Welp, can't reproduce those today after reimporting everything from scratch.
I opened #42569 for this. |
akien-mga
added a commit
to akien-mga/godot
that referenced
this issue
Oct 5, 2020
The glTF 2.0 spec only makes `mimeType` mandatory for `bufferView` image data, so the previous logic to handle URIs with base64-encoded images could fail if `mimeType` is undefined. The logic was documented and refactored to better handle the spec, notably: - `uri` and `bufferView` are now mutually exclusive, and only the latter fails if `mimeType` is undefined. - `uri` with a file path will now respect the `mimeType` if defined, and thus attempt loading the file with the specified format (even if its extension is not the one expected for this format). So we can support bad extensions (PNG data with `.jpg` extension) or custom ones (PNG data in `.img` file for example). - `uri` with base64 encoded data will infer MIME type from `data:image/png` or `data:image/jpeg` if it was not documented in `mimeType` initially. - `uri` with base64 encoded data, no `mimeType` and `application/octet-stream` or `application/gltf-buffer` will fall back to trying both PNG and JPEG loaders. Fully fixes godotengine#33796 (and fixes up godotengine#42501).
akien-mga
added a commit
that referenced
this issue
Oct 5, 2020
…* MIME types See KhronosGroup/glTF#944 for context on the application/gltf-buffer MIME type. The glTF 2.0 spec supports `image/jpeg` and `image/png` which can also be base64-encoded in buffer URIs. Fixes #33796. (cherry picked from commit 34a5031)
akien-mga
added a commit
that referenced
this issue
Oct 5, 2020
The glTF 2.0 spec only makes `mimeType` mandatory for `bufferView` image data, so the previous logic to handle URIs with base64-encoded images could fail if `mimeType` is undefined. The logic was documented and refactored to better handle the spec, notably: - `uri` and `bufferView` are now mutually exclusive, and only the latter fails if `mimeType` is undefined. - `uri` with a file path will now respect the `mimeType` if defined, and thus attempt loading the file with the specified format (even if its extension is not the one expected for this format). So we can support bad extensions (PNG data with `.jpg` extension) or custom ones (PNG data in `.img` file for example). - `uri` with base64 encoded data will infer MIME type from `data:image/png` or `data:image/jpeg` if it was not documented in `mimeType` initially. - `uri` with base64 encoded data, no `mimeType` and `application/octet-stream` or `application/gltf-buffer` will fall back to trying both PNG and JPEG loaders. Fully fixes #33796 (and fixes up #42501). (cherry picked from commit 2e99d0b)
MarcusElg
pushed a commit
to MarcusElg/godot
that referenced
this issue
Oct 19, 2020
…* MIME types See KhronosGroup/glTF#944 for context on the application/gltf-buffer MIME type. The glTF 2.0 spec supports `image/jpeg` and `image/png` which can also be base64-encoded in buffer URIs. Fixes godotengine#33796.
MarcusElg
pushed a commit
to MarcusElg/godot
that referenced
this issue
Oct 19, 2020
The glTF 2.0 spec only makes `mimeType` mandatory for `bufferView` image data, so the previous logic to handle URIs with base64-encoded images could fail if `mimeType` is undefined. The logic was documented and refactored to better handle the spec, notably: - `uri` and `bufferView` are now mutually exclusive, and only the latter fails if `mimeType` is undefined. - `uri` with a file path will now respect the `mimeType` if defined, and thus attempt loading the file with the specified format (even if its extension is not the one expected for this format). So we can support bad extensions (PNG data with `.jpg` extension) or custom ones (PNG data in `.img` file for example). - `uri` with base64 encoded data will infer MIME type from `data:image/png` or `data:image/jpeg` if it was not documented in `mimeType` initially. - `uri` with base64 encoded data, no `mimeType` and `application/octet-stream` or `application/gltf-buffer` will fall back to trying both PNG and JPEG loaders. Fully fixes godotengine#33796 (and fixes up godotengine#42501).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Godot version:
3.2.beta.custom_build. 083d088
OS/device including version:
Ubuntu 19.10
Issue description:
After importing minimal project there is a lot of errors but I saw that Godot try to use encoded base64 text as path to files.
or very long(> 50000 characters)
Minimal reproduction project:
https://github.com/KhronosGroup/glTF-Sample-Models/archive/master.zip
The text was updated successfully, but these errors were encountered: