-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add struct chaining docs #469
Conversation
Also test the thing in the example text. Issue: none, 264, should have been in 25, related to 463
They occur if and only if: | ||
|
||
- The `sType` of a struct in the chain is not valid for the type of the chain root. | ||
- Multiple instances of the same `sType` value are seen in the same struct chain. (Note this also detects and disallows cycles.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit. To clarify this a bit more (at least w.r.t the fuzzer case), it's not just that the same sType
can't be in the same chain, but that the same sType
should never appear in a chain or member chain on a root struct. Not sure how to word it clearly here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(discussed offline)
The first requirement (sType is valid for the type of the chain root) should be the one that validates that. The only gap is if some implementation adds an extension that can actually recurse, which we probably should not allow them to do, so I'll note that.
We should also guarantee that if the implementation doesn't allow the sType in that context, it will never try to downcast the struct. It's the same as if the sType were totally bogus.
In Dawn's wire, Loko came up with a way to guarantee that invalid chains can be serialized and deserialized without hitting serialization errors: if the client doesn't allow an sType in context, convert it to something like WGPUSType_DawnInvalidChainedStruct
which indicates struct WGPUDawnInvalidChainedStruct { WGPUChainedStruct chain; WGPUSType sTypeForErrorMessage; }
. As long as the client and server are in sync the server can always handle this without a deserialization error, and forward it into dawn-native which will produce a validation error. If the client is out of sync or is malicious (e.g. a fuzzer) then if the server sees an sType that's not valid in context it immediately gives up and fails deserialization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only gap is if some implementation adds an extension that can actually recurse, which we probably should not allow them to do, so I'll note that.
Actually I started trying to write this down and realized there could be legitimate use cases that pass variable-sized data as a linked list (rather than an array or array of pointers), regardless of whether they're using struct chaining. So I'm going to make this a "should", not a "must".
Also add compile tests for the things I wrote in the documentation sample code.
Issue: none, #264, should have been listed in #25, related to #463