w3c / wot-scripting-api

Web of Things (WoT) Scripting API
http://w3c.github.io/wot-scripting-api/
Other
42 stars 28 forks source link

Non-validating value() function #554

Open danielpeintner opened 4 months ago

danielpeintner commented 4 months ago

Based on the discussion in https://github.com/eclipse-thingweb/node-wot/pull/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

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 commented 4 months ago

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 commented 3 months ago

@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 commented 2 months ago

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 commented 2 months ago

I can live with it also 👍

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

zolkis commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Call 2024-08-05:

TallTed commented 2 months ago

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 commented 2 months ago

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 commented 4 weeks ago

@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 commented 4 weeks ago

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 commented 4 weeks ago

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.