Validation provider doesn't decode external URIs before calling fs.readFile

[lexaknyazev]

Consider an asset like this:

{
    "asset": {
        "version": "2.0"
    },
    "buffers": [
        {
            "uri":"spaces in name.bin",
            "byteLength": 1
        }
    ]
}

externalResourceFunction will be called with spaces%20in%20name.bin (because it's an URI), and fs.readFile will fail.

[donmccurdy]

Related to KhronosGroup/glTF#1100 — Suppose instead that the asset had a "?" in the filename, and a query parameter:

/path/to/filename-with-%3F-question-mark.bin?and=query-params

It's not intended that the query parameter be URI-encoded; that should be passed un-escaped to externalResourceFunction. The ? in the filename must be escaped in the asset, or it is invalid. So I think that technically, spaces in the URI should also be escaped in the asset itself.

Although, spaces in particular are not a big problem for the web, as this code will automatically have spaces converted to %20 correctly:

fetch('https://www.example.com/?q=here is a query')
  .then((response) => response.text());

[donmccurdy]

More to the point, is it an invalid URI if it contains a space?

[emackey]

Good question. I would think ideally we'd want glTF to be somewhat tolerant, like the web is. So an encoded space is official, but a non-encoded space is still functional.

Query parameters on relative URIs are more questionable in my mind, since that's an HTTP protocol thing, and the root URI may be file or ftp or similar with no support for query parameters. The glTF itself may not know its own root path (hence that's passed in separately to most engines).

Obvious but worth noting that this doesn't affect "Embedded" files or the typical GLB that follows the embedded pattern, since the needed data is in the same file. It's only an issue for files that are broken into pieces (which is sometimes an indication the file is still being edited).

[donmccurdy]

I would think ideally we'd want glTF to be somewhat tolerant, like the web is. So an encoded space is official, but a non-encoded space is still functional.

Agreed.

EDIT: More specifically, I agree tools should strive to support spaces. Most everything on the web already will. But I don't think we should loosen the spec to say "The "uri" field must be a valid URI EXCEPT that it can also contain spaces". It should just be a rule that we're lax about in the validator; maybe a hint or info, or no message at all.

[lexaknyazev]

More to the point, is it an invalid URI if it contains a space?

It's a complex question. Officially, it shouldn't contain spaces, but there're some normalization rules to fix that on the fly.

This issue is aimed at fixing fs.readFile call.

[donmccurdy]

Sure. I have no problem with the PR, feel free to merge that. But (now or later) I think we should think through whether the validator should be URI-encoding what is already supposed to be a valid URI.

[emackey]

Fixed in 2.1.2