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

Non-validating value() function #554

Closed
danielpeintner opened this issue Jun 3, 2024 · 13 comments
Closed

Non-validating value() function #554

danielpeintner opened this issue Jun 3, 2024 · 13 comments
Assignees

Comments

@danielpeintner
Copy link
Contributor

Based on the discussion in eclipse-thingweb/node-wot#1279 two points/issues arose. One is the following.


The current value() function of InteractionOutput requires as step 10 to validate the value.

I think there are good use-cases to avoid doing validation

  • performance
  • getting the value even if it does not validate
  • ...

Note: I think the current requirement to MUST validate the data reported is also very heavy in a sense. I think there are (and will be) implementations out there that simply do not have the power to validate the data (no JSON schema library available etc).
I wonder how we can account for that? Say it is best effort only?

Anyhow, in the thingweb committer call we had the feeling that we should at least extend the current value function in a way that the user of the API can decide whether validation is done or not. This could be done in a backwards compatible way changing the current signature of

Promise<DataSchemaValue> value();

to something like

Promise<DataSchemaValue> value(optional ValueOptions = {});

dictionary ValueOptions {
  boolean validate; /* default?*/
};

Yet another question is what the default behavior of validate should be?

@relu91
Copy link
Member

relu91 commented Jun 4, 2024

One thing to consider is that we designed the InteractionOutput interface inspired by Body Mixin type from the Fetch standard. There, as you know, there is already a function defined with a purpose similar to the value function minus the validation. Therefore, we might consider to even introduce yet another function inside the InteractionOuput taking the description from the json().

About default, I think that the function should validate as the current behavior is always validated. Changing this would be a very sneaky move. Image that you are expecting the function to always return well formed data and later in mid of your application you find that a field is missing or is an number instead of a boolean.

@danielpeintner
Copy link
Contributor Author

@relu91 If I read your comment correctly you suggest the following way forward

  1. keep value() as is with the nature of validation (no change to the signature)
  2. introduce json() as yet another method that reports the value without validation

I am fine with this way forward even though I think it is clearer to have value({"validate":false}) instead...

@relu91
Copy link
Member

relu91 commented Jul 22, 2024

Yes, I'm suggesting that path forward but I'm open to other opinions too, I think in the end it's a matter of taste and developer background. For web developers looking for the json function would be automatic (and they may like the guarantees of the value function).

@danielpeintner
Copy link
Contributor Author

I can live with it also 👍

@zolkis @JKRhb others
Are you okay with it also?

@zolkis
Copy link
Contributor

zolkis commented Jul 22, 2024

In a browser API there would be no way to skip validation, for security reasons.
But this is not a browser API.
Anyway, a decision to allow skipping validating should include publishing the rationale in the spec and a security threats/mitigations section.

@relu91
Copy link
Member

relu91 commented Jul 23, 2024

Skipping validation is already possible, the newly introduced function (or option) would just be a shortcut for:

// output is an InteractionOutput instance
const value = JSON.parse(Buffer.from(await output.arrayBuffer()).toString())

But I agree that adding text about possible threats regarding the practice of avoiding validation is something we should do.

@JKRhb
Copy link
Member

JKRhb commented Jul 23, 2024

I can live with it also 👍

@zolkis @JKRhb others
Are you okay with it also?

Sounds good to me :) I am only wondering if we should choose a different name than json() since we could potentially have other content types as well.

@relu91
Copy link
Member

relu91 commented Aug 5, 2024

Call 2024-08-05:

  • @danielpeintner agrees that json() is not the right name, mabye raw() ?
  • @ashimura there are implications about validation in TD, we should verify it. @relu91 will take a look.

@relu91 relu91 self-assigned this Aug 5, 2024
@TallTed
Copy link
Member

TallTed commented Aug 5, 2024

Call 05/09

I'm pretty sure you meant August 5 (not May 9, not September 5). Best to use ISO8601 format, e.g., 2024-08-05, especially but not only for such potentially ambiguous dates.

@relu91
Copy link
Member

relu91 commented Aug 6, 2024

I'm pretty sure you meant August 5 (not May 9, not September 5). Best to use ISO8601 format, e.g., 2024-08-05, especially but not only for such potentially ambiguous dates.

Thanks! updated.

@relu91
Copy link
Member

relu91 commented Sep 16, 2024

@ashimura there are implications about validation in TD, we should verify it. @relu91 will take a look.

On this topic, I went back to looking at the latest TD recommendation document and I found this assertion:

A WoT Thing Description MUST accurately describe the data returned and accepted by each interaction.

If I read it right this means that the cases where we are "getting the value even if it does not validate" are buggy, meaning that we are perpetuating something already wrong. For reference that section adds also some other assertions on how Consumers should behave when interacting with a remote Thing:

  • A Consumer when interacting with another target Thing described in a WoT Thing Description MUST generate data organized according to the data schemas given in the corresponding interactions.
  • A WoT Thing Description MUST accurately describe the data returned and accepted by each interaction.
  • A Thing MAY return additional data from an interaction even when such data is not described in the data schemas given in its WoT Thing Description. This applies to ObjectSchema and ArraySchema (when items is an Array of DataSchema) where there can be additional properties or items in the data returned. This behaves as if "additionalProperties":true or "additionalItems":true as defined in [JSON-SCHEMA].
  • A Consumer when interacting with another Thing MUST accept without error any additional data not described in the data schemas given in the Thing Description of the target Thing. This applies to ObjectSchema and ArraySchema (when items is an Array of DataSchema) where there can be additional properties or items in the data returned. This behaves as if "additionalProperties":true or "additionalItems":true as defined in [JSON-SCHEMA].
  • A Consumer when interacting with another Thing MUST NOT generate data not described in the data schemas given in the Thing Description of that Thing.
  • A Consumer when interacting with another Thing MUST generate URIs according to the URI Templates, base URIs, and form href parameters given in the Thing Description of the target Thing.
  • URI Templates, base URIs, and href members in a WoT Thing Description MUST accurately describe the WoT Interface of the Thing.

I think we might still have the "performance" use case for introducing the raw function and the assertions above could help to build the security/treats section that @zolkis was talking about.

@relu91
Copy link
Member

relu91 commented Sep 16, 2024

Given the new information above, we might want to reconsider not introducing a new function but rather mention the alternatives like this one. I'll work on a concrete PR.

@egekorkan
Copy link
Contributor

If I read it right this means that the cases where we are "getting the value even if it does not validate" are buggy, meaning that we are perpetuating something already wrong.

I agree with this. The only possible cases are unexpected states of a Thing where the TD would explode trying to describe all possible error values and the TD designer takes a "shortcut" by providing an underspecified TD.

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

No branches or pull requests

6 participants