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

Handling unsupported extensions #695

Closed
emackey opened this issue Aug 25, 2016 · 28 comments
Closed

Handling unsupported extensions #695

emackey opened this issue Aug 25, 2016 · 28 comments

Comments

@emackey
Copy link
Member

emackey commented Aug 25, 2016

How should user agents handle models that declare unknown or unsupported extensions?

Currently, Cesium scans the extensions list and will throw if it finds anything it doesn't recognize. This is problematic for example with glTF models exported from the Blender exporter, which define some as-yet-unused Blender-specific extensions, all of which should be perfectly safe for Cesium to disregard.

My proposal is that glTF should specify that user agents follow the typical web behavior of making a best-effort to display the provided content, and not fail due to the mere presence of unknown extensions. Likewise, glTF authoring software should strive to produce models that are at least minimally useful (or at least valid glTF) even if the extension block(s) were completely removed from the file.

In the case of the BLENDER_Physics extension, this is trivial: Blender encodes settings needed by its physics engine into the model, and rendering engines are free to display the model outside of any physics engine, safely ignoring the unknown extension.

Things get more complicated with proposed material extensions. Ideally, a glTF author could provide a simple fallback material, of lower visual quality, to be a stand-in for rendering engines where the high-quality material extension is unavailable. If this is not practical, I believe there may be a default (unlit gray) rendering strategy that could be used in lieu of the missing material extension.

@lexaknyazev
Copy link
Member

My proposal is that glTF should specify that user agents follow the typical web behavior of making a best-effort to display the provided content, and not fail due to the mere presence of unknown extensions.

"Best-effort" result may vary depending upon the extension. E.g., what should runtime do with accessors having (unsupported) WEB3D_quantized_attributes?

Maybe, an asset could declare some extensions as "mandatory" or "recommended" to ensure desired compatibility / interoperability level.

@emackey
Copy link
Member Author

emackey commented Aug 26, 2016

Maybe, an asset could declare some extensions as "mandatory" or "recommended"

I like this idea so long as the "mandatory" ones must declare themselves to be so (by whatever means, inside the glTF). I think all of the existing known extensions are non-mandatory, and the default behavior should be to proceed with rendering.

I'd also be OK with not actually specifying "mandatory" and just allowing the throw to happen if and when things like unsupported accessors are discovered. I know some find this behavior sloppy, but this model has served the web for many years: Pages load with broken images, missing stylesheets, etc., and the browser keeps trying to do its best with whatever it supports. This strategy has given the web a vast reach, allowing it to work on devices and in environments that web developers didn't expect.

@lexaknyazev
Copy link
Member

I'd also be OK with not actually specifying "mandatory" and just allowing the throw to happen if and when things like unsupported accessors are discovered.

In case of quantized accessors there is no way to predict correctness of initial rendering, because glTF asset is still valid with extension object removed. If a runtime runs into unsupported extension, it can't know how that extension affects model.

Pages load with broken images, missing stylesheets, etc., and the browser keeps trying to do its best with whatever it supports.

On the other hand, modern web apps still contain polyfills, browser-specific checks and quirks for "active" content (layout & scripts).

Support for media files is more straightforward: e.g., user-agent either supports WebP/Vorbis/AVC or not. Since glTF aims to be a "JPEG-for-3D", I think its behavior should be as predictable as possible, however nothing should prevent a model from rendering if user-agent doesn't support physics extension.

@emackey
Copy link
Member Author

emackey commented Aug 26, 2016

Hmm, OK, so, what should the proposal look like now? There's an extensionsUsed array in glTF that has the complete list of extensions used, including optional ones. Should there be an extensionsRequired array in addition to this? Or some better way of indicating which ones are required?

Another thought, is there any need to specify the "optional" extensions used at all? The required ones are listed just so the rendering engine can find them and abort early. But do the optional ones need to be listed? If not, maybe we could get rid of extensionsUsed and just have requiredExtensions or such?

@lexaknyazev
Copy link
Member

Another thought, is there any need to specify the "optional" extensions used at all? The required ones are listed just so the rendering engine can find them and abort early. But do the optional ones need to be listed?

Besides new objects, extensions can introduce new parameter semantics and reserved ids. So runtime needs to know extension full name to properly handle them.

@emackey
Copy link
Member Author

emackey commented Aug 26, 2016

OK, but I think we want to encourage new extensions like BLENDER_Physics to be introduced without breaking the glTF reader ecosystem as it does now. How can we fix this?

@lexaknyazev
Copy link
Member

Currently, I can see several non-perfect ways:

  • Make two lists of extensions
  • Define more complex naming convention (e.g., prefix for optional extensions)
  • Define fallback behavior for each object type (e.g., default material for material, but throw for accessor)

Thoughts?

@javagl
Copy link
Contributor

javagl commented Aug 27, 2016

This seems to be related to the "backward compatibility" that I mentioned in #646 (comment) . E.g. if I understood this correctly, then this BLENDER_Physics seems to be a pure addition to an (otherwise valid) glTF file, whereas a glTF that uses binary_glTF is plainly invalid for a loader that doesn't know the extension.

I think that there has to be some sort of classification, but I'm also not sure about the best way.

Right now, one could say that the entry in extensionsUsed means that there must be a certain awareness of the extension by the loader. Other "unspecified additional information", like physics parameters, could silently be inserted into the extensions of the respective object, and would automatically be ignored by unaware loaders. More specifically: Consider a loader that knows that there may be some physics information associated with, for example, a meshPrimitive. What information would it gain by finding the entry BLENDER_Physics in the extensionsUsed? It still would have to look into the extensions of each meshPrimitive, so the information from the extensionsUsed is at this point basically useless.

I think that the third option (defining the behavior for each object type) would open a whole new can of worms by dramatically increasing the complexity of the spec, but maybe I'm overestimating this.

(A more general form of this question is about the resiliency of loaders. For example, when a texture IDs is invalid (or its image can not be loaded), then the loader could just bail out and say "This is an invalid file, better luck next time". Or it could try to show the mesh un-textured. Or it may throw/crash. In how far should the spec constrain the options here?)

@lexaknyazev
Copy link
Member

Not listing all used extensions can lead to confusion in non-trivial cases, because nothing prevents different extensions from defining same ids or semantics.

A more general issue here is extension compatibility, see #696 (comment), #658.

@pjcozzi
Copy link
Member

pjcozzi commented Aug 29, 2016

I thought this was discussed a few years ago and that we considered separate lists for required (e.g., quantization) vs. optional (e.g., physics, collision volumes, etc.), but I can't find the issue. Given the use cases we have so far, I still feel like that is the best solution.

@lexaknyazev is this something you want to consider for 1.0.1? I feel it is important as the glTF ecosystem grows (especially with Blender export!!!), but I also want to contain the 1.0.1 scope so we can finish soon.

Also, note that - in the short-term - Cesium could work around this by ignoring or implementing the extension.

@emackey
Copy link
Member Author

emackey commented Aug 29, 2016

I want to respond to a question from @javagl here:

(A more general form of this question is about the resiliency of loaders. For example, when a texture IDs is invalid (or its image can not be loaded), then the loader could just bail out and say "This is an invalid file, better luck next time". Or it could try to show the mesh un-textured. Or it may throw/crash. In how far should the spec constrain the options here?)

I believe that glTF loaders should be as resilient as possible. If the texture isn't there, rendering an untextured color is preferable to complete failure (just as rendering HTML with a broken image link is preferable to not showing a page at all). I'm not saying we need a ton off fallback code, just a reasonable effort to keep going (for example, supplying a blank texture in place of the missing one).

We might not actually need a spec change here, beyond a note about how readers should handle unknown extensions: I think they should ignore them. At best, report a console warning, but don't throw based on the mere presence of unknowns in extensionsUsed. If the extension was truly required, then something will eventually break someplace, but I don't think the reader needs to go to any extra effort to make that breakage happen any sooner than it needs to. Perhaps if an exception is caught later, then the reader could optionally go back looking for unknown extensions in use, and display its findings along with the error message. Or not. But don't preemptively abort the glTF loading just because the loader found something it didn't recognize; that's not resilient enough for use on the web.

@javagl
Copy link
Contributor

javagl commented Aug 29, 2016

Hoping that the broader question is not too off-topic for this issue:

One always has to expect corrupt input, particularly when the input is coming over the wire, live, as it is the case for glTF. But achieving such a resiliency may involve different efforts - in general, and regarding the particular question of this issue, namely the unknown extensions:

If the extension was truly required, then something will eventually break someplace, but I don't think the reader needs to go to any extra effort to make that breakage happen any sooner than it needs to.

I'm not so familiar with JavaScript, but I think the worst thing that can happen there is a console message. For Java, the same error could end the application with an exception. For C++, any out-of-bounds access is allowed to initiate a zombie apocalypse.

I ended up with doing some basic validation, and bailing out with a (hopefully helpful) message before even trying to render the glTF. Of course, a more forgiving and resilient approach would be preferable. But I noticed that painstakingly wrapping everyhing into if (somethingRequiredIsMissing) tryToCopeWithIt() statements is hardly feasible, and, to some extent, defeats the purpose of declaring something as "required" in the spec...

@lexaknyazev
Copy link
Member

For C++, any out-of-bounds access is allowed to initiate a zombie apocalypse.

To prevent zombie apocalypse, active content (such as glTF) could be treated like program binaries. Binary glTF file could be easily extended with PKI signature. Such signature could be added by trustworthy authority (e.g., Khronos-hosted web service) only if an asset passes the validation. Runtime could check asset's signature if data came from unknown source.

@emackey
Copy link
Member Author

emackey commented Aug 29, 2016

I think the worst thing that can happen there is a console message

Throwing an exception from JavaScript can break your WebGL render loop, locking up at least the 3D portion of your page. With the current Cesium loader, this lockup happens in response to the mere naming of unknown extensions as being in use.

To bring this back on topic, I'm not suggesting adding a bunch of new tryToCopeWithIt() logic, I'm just suggesting the loader (at least JS/Java/C# loaders) shouldn't throw early. As for C++ and the zombie apocolypse, sure, they should do much more stringent sanity checking, since that's not being taken care of by any runtime or managed layer between them and the OS.

@javagl
Copy link
Contributor

javagl commented Aug 29, 2016

To prevent zombie apocalypse, active content (such as glTF) could be treated like program binaries. Binary glTF file could be easily extended with PKI signature. Such signature could be added by trustworthy authority (e.g., Khronos-hosted web service) only if an asset passes the validation. Runtime could check asset's signature if data came from unknown source.

That sounds like a sophisticated approach, but (although I'm not familiar with PKI mechanisms) like it could be difficult to apply this, for example, to glTF content that is generated "in realtime" (as the direct response of a browser request).

Throwing an exception from JavaScript can break your WebGL render loop, locking up at least the 3D portion of your page.

I see. This sounds like a case where the error could be handled with little effort, to avoid a severe problem. Beyond that, the options for the exact behavior in case of errors always involve the usual trade-offs (implementation effort, severity, and maybe likelihood of the error...)

@lexaknyazev
Copy link
Member

@lexaknyazev is this something you want to consider for 1.0.1? I feel it is important as the glTF ecosystem grows (especially with Blender export!!!), but I also want to contain the 1.0.1 scope so we can finish soon.

One more simple root property will do no harm. The question is how to divide extensions

  • all extensions + required extensions or
  • optional extensions + required extensions

First approach is more compatible, while the second is more explicit.

@lexaknyazev
Copy link
Member

That sounds like a sophisticated approach, but (although I'm not familiar with PKI mechanisms) like it could be difficult to apply this, for example, to glTF content that is generated "in realtime" (as the direct response of a browser request).

Malicious glTF asset published on a popular model-sharing site could affect many users, so keeping ecosystem safe should be on the list of glTF goals.

Speaking of runtime resilience (preventing crashes), I think there are at least three different usage scenarios:

  1. You own content or you live-stream it from your (or other trusted) servers to your application. In this case, there is no need for any validation layer except origin check. Web has time-proven ways (TLS, CORS, etc) to ensure it.
  2. You use 3-rd party assets (provided by dynamic URL) in your app. In this case, the only way to avoid validation is to have an asset validated by someone else, who you trust (and digital signature seems to be a proper way to do it).
  3. You process user-provided assets (think of Sketchfab). Full validation is unavoidable. Valid files could be self-signed afterwards.

Doing not-so-full validation will, imho, reduce performance on valid assets, while keeping runtime vulnerable to invalid files.

@emackey
Copy link
Member Author

emackey commented Aug 31, 2016

Malicious glTF asset published on a popular model-sharing site could affect many users, so keeping ecosystem safe should be on the list of glTF goals.

Totally agree, but the needs of the web are very different from the needs of C++ here. WebGL already specifies quite a bit of robust index-out-of-bounds checking and protections from malicious content, in a way that's going to be hard for C++ to replicate. Since glTFs don't include their own HTML/CSS/JS, I don't know if or how a malicious glTF could be used for cross-site scripting, for example. Ideally the underlying WebGL implementation would do its job protecting from uninitialized memory, buffer overflows, index out of bounds, etc. We can never rule out a possible security issue, but in the case of the web at least a lot of thought has been put into WebGL's protections.

C++ apps don't have WebGL's layers of protection, and gain a performance boost by avoiding that. But these apps are especially sensitive to invalid memory access. It seems likely that these apps would want some source of trusted glTF files (meaning, supplied by the app's author), or digitally signed glTF files.

Even so, I see this discussion as being off-topic with regards to how to handle unknown extensions. Finding a glTF with an unknown extension in use isn't cause for security alarm, if the security issues surrounding intentionally malformed glTFs have been addressed separately.

@pjcozzi
Copy link
Member

pjcozzi commented Sep 10, 2016

@lexaknyazev could you please submit a roadmap item to your validator repo about PKI and security? We can definitely consider it for a 2.0 validator.

One more simple root property will do no harm. The question is how to divide extensions

  • all extensions + required extensions or
  • optional extensions + required extensions

First approach is more compatible, while the second is more explicit.

I agree with @lexaknyazev here that this is the right approach (probably optional extensions + required extensions since I suspect Cesium is one of the only loaders that actually checks).

If it's required and a loader doesn't implement it, they can bail without creating undue harm. If it is optional, they can warn if they want, and carry on.

If we can reach consensus on this, I would encourage us to include it in 1.0.1, but that is up to @lexaknyazev as I don't want to overly increase the scope.

@lexaknyazev
Copy link
Member

probably optional extensions + required extensions since I suspect Cesium is one of the only loaders that actually checks

Should we then rename extensionsUsed? Such name fits nor "required" neither "optional".

If it's required and a loader doesn't implement it, they can bail without creating undue harm. If it is optional, they can warn if they want, and carry on.

+1

If we can reach consensus on this, I would encourage us to include it in 1.0.1.

+1

@pjcozzi
Copy link
Member

pjcozzi commented Sep 10, 2016

Should we then rename extensionsUsed? Such name fits nor "required" neither "optional".

Yes, I think this breaking change is OK for 1.0.1 since this is not widely used.

@javagl @emackey any final thoughts on this?

@javagl
Copy link
Contributor

javagl commented Sep 10, 2016

I think "used" is neutral enough to convey that the contents of extensionsUsed is an informative list (i.e. that a support of these extensions is "optional"). One could even allow it to contain ALL extensions, regardless of whether they are required or not

"extensionsUsed" : [ "theRequiredOne", "anOptionalOne" ],
"extensionsRequired" : [ "theRequiredOne" ],

But I don't have a strong opinion here. There are probably not many backward compatibility issues involved, and if a dedicated list of the optional ones is considered as a cleaner solution, then that's fine.

@emackey
Copy link
Member Author

emackey commented Sep 13, 2016

I like @javagl's proposal. But just to think this through a little more: What's the difference between an "optional" extension vs. use of the 'extras' object? Could we say that all "optional" extensions would be better implemented as a child object of extras rather than a named extension? This would let us keep the current extensionsUsed as a list of required extensions. I realize this is a very different line of thinking than I had when I first opened this issue.

One of COLLADA's drawbacks (for our STK desktop software, and possibly others) is that the "coolest" features (shaders, STK articulations, etc) are all stored in extensions that aren't parsed by other vendors' code. Perhaps we should go the other direction for glTF: Encourage implementors not to make up their own extensions, and try to stick to the core spec and only extensions listed in this repo. This could make glTF much more interoperable. Apologies if I'm sending us on a 180 here, someone was just asking me this morning about our troubles with COLLADA, and I realized I might be pushing glTF in the same direction.

@RemiArnaud
Copy link
Contributor

Since you mention COLLADA extension mechanism, I want to state that there is a big difference in design goal between the two formats, that directly impact this topic of extension - as well as other topics.

COLLADA design goal is to carry all information between tools in an content pipeline. Not every tool can understand all the data - extensions or not - and the format is designed such that it is possible to carry forward information easily even if the tool has no idea what it is about.

GLTF is an endpoint format, there is no transmission of data from Gltf to another Gltf. In fact, there is absolutely no need to transmit extensions/extras in Gltf to a client that does not need it. Gltf is best provided by a server that can map client requests to URL and provide exactly what is required, just like other transmission formats such as HTML.

@pjcozzi
Copy link
Member

pjcozzi commented Sep 13, 2016

What's the difference between an "optional" extension vs. use of the 'extras' object?

The extra property is more for application-specific metadata, e.g., you might store strings there that you want to use for display in a UI.

Required and optional extensions are fully specified. We encourage any potentially generally useful glTF feature to be packaged as an extension, not as "application-specific" properties in extras.

Extensions and extras were intended to be separate features and I still think they is OK even with optional extensions; we just never thought through optional vs. required extensions until now.

@emackey
Copy link
Member Author

emackey commented Sep 13, 2016

OK, so given that optional extensions DO exist, I'll go back and say +1 for @javagl's proposal:

"extensionsUsed" : [ "theRequiredOne", "anOptionalOne" ],
"extensionsRequired" : [ "theRequiredOne" ],

@lexaknyazev
Copy link
Member

+1 for extensionsUsed + extensionsRequired.

@pjcozzi are you OK with this? I'll make a PR.

@pjcozzi
Copy link
Member

pjcozzi commented Sep 16, 2016

@lexaknyazev go for it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants