eclipse-thingweb / node-wot

A fast and extensible framework to connect any device with your browser and backend applications
https://thingweb.io
Other
166 stars 80 forks source link

Handle form `content-type` client side #854

Closed relu91 closed 6 months ago

relu91 commented 2 years ago

Hi all. I'm opening this issue as a consequence of the latest discussion during the TD/Protocol binding templates call. During the call, people complained about how node-wot clients (primarily HTTP and COAP) are ignoring the content-type field in forms. Sadly I couldn't find the minutes of the call so I'm trying to summarize by heart.

Looking at https://github.com/eclipse/thingweb.node-wot/blob/master/packages/binding-http/src/http-client-impl.ts, what I see, is that the content type is taken into account for every write operation (i.e., invokeaction and writeproperty), whereas is ignored in reading operations (node-wot uses what the server gives). Do people feel like that node-wot HTTP client should set the Accept header to the content-type value when reading? To me is quite redundant but it does not hurt that much. Do you have a compelling reason for doing so?

I know that the COAP discussion is more controversial due to mappings between coap-specific fields and WoT content-type, so I would leave it after we have a common take on HTTP.

pinging relevant people: @egekorkan @mkovatsc @danielpeintner @hidetak @JKRhb. If I recalled something wrong feel free to correct me, I'll update this first post accordingly.

egekorkan commented 2 years ago

I'd say that it should set accept header since it can be that there are two identical forms, only differing in contentType.

mkovatsc commented 2 years ago

The purpose of the contentType field (and contentCoding where applicable) is to inform the upper layers what format comes out / goes into this particular interaction using this form. op is used to match a form to the interaction intended by the application. All other fields of the form have the purpose to describe how to generate a request that results in exactly the behavior the application expects and the Thing is able to understand, without any negotiations left open -- within a form, all negotiations are decided at design time.

There still is the concept of content negotiation, as it is part of the Web Architecture. It is not happening at protocol binding level, however, yet at the form level: when a Thing provides N different formats, the TD author can include N different forms, each with a different contentType, so that WoT Servients can pick a format for which they have a handler implemented. (This is why the field must not be moved up to the affordance level!) Content format handlers are between bindings and affordances, so they can be used across all bindings. Content format strings are WoT's abstraction mechanism to deal with any data format on the wire; some protocols already use them, for others we need to define them like the URI schemes, both resulting in uniform interfaces for these protocols (modulo the ever-missing HATEOAS part...).

The verbose but strict solution would now be to have the contentType field for the upper layers and another field that ensures the request yields or communicates that format in the request. For HTTP, it would be a full duplication in the Accept header definition; for CoAP, it would be the Content Format number in its definition field. This redundancy can be optimized when there are clear rules how the contentType translates to the specific binding (trivial for HTTP, for CoAP contentType anyway must be out of the list of registered Content-Format numbers, others might need a lookup table provided with the binding). This is how the original node-wot implementation did it for the http-binding.

Revisiting this, however, I now notice that this optimization loses the information if the Thing actually needs this information in the request, i.e., provides multiple formats, or not. If this information should be preserved, we may keep the duplication of contentType into its corresponding protocol header -- they still mean two different things (inform upper layers vs description how exactly to generate the request). Furthermore, I have the feeling that all these optimizations in TD rather cause confusion. So maybe we should make it verbose, but formal. The absence of the protocol-specific half would carry the information, the Thing does not have content negotiation and always responds with something that matches the contentType as the TD was designed that way.

mkovatsc commented 2 years ago

Pinging @ektrah as this relates to https://github.com/w3c/wot-binding-templates/pull/188

relu91 commented 1 year ago

@egekorkan @mkovatsc I think we are on the same page. Some points:

Furthermore, I have the feeling that all these optimizations in TD rather cause confusion. So maybe we should make it verbose, but formal. The absence of the protocol-specific half would carry the information, the Thing does not have content negotiation and always responds with something that matches the contentType as the TD was designed that way.

As always it would be nicer to have more compact TDs, even more, when the mapping is kind of obvious for particular bindings (even if @egekorkan's table proves the contrary due to dependencies of op and new form attributes). However, as you said, I agree that these optimizations are the source of confusion (even for ourselves). If we keep everything explicit we have to answer these kinds of client implementation questions:

  1. What happens if in response to a "read" operation the client detects a content-type different from what is declared in the selected form?
    1. In some cases the form is selected automatically by the runtime, should we search for the correct form with the right content type in the affordance form list? What happens if we can't find the right form?
  2. If a TD designer has to be verbose about how the request is issued it means that there might be cases where he declares wrong information (i.e., accept requires a different media type of what contentType declares). Is this a warning? or should we throw a validation error upon consuming?
  3. If the TD designer forgets to add verbose information what the runtime should do? Automatically fixing means to fallback into a non-verbose where we assume defaults mappings.
egekorkan commented 1 year ago

My answers:

  1. I think that some warning-level log can be shown and everything goes as usual IF node-wot is able to decode the payload. If it cannot, an error-level log should be shown. This can be problematic in the case where the Consumer chooses a form (manual or automatic) based on the knowledge of the contentType support of node-wot but then gets another contentType. I think this is simply a violation of the contract (the TD) so we should not make the implementation more complex due to this. It is in the same level as getting a 404 for an HTTP form.
  2. This should be a validation error upon consuming. Not sure if this can be done via a JSON Schema.
  3. I did not understand this case 100% . I think that the form contentType of a response can be ignored if there is a contentType header in the protocol response.
mkovatsc commented 1 year ago

HI all, here are consolidated answers from @ektrah and myself:

  1. Content-Type differs:
    • Why does it differ? There should only be two cases (excluding byzantine Things..):
      • TD is wrong, including that necessary protocol instructions such as an Accept header are missing
      • TD is out-of-date --> may try to fetch it again, if it originated from the Thing directly
    • Consumer must use protocol in-band information to choose a content type handler and must not apply TD value for Content-Type if it differs; if the protocol does not have means for content type indication, there can be no conflict and Consumer has to rely on TD
    • Consumer may trigger an error and abort
    • Consumer may resolve by looking for a matching handler and continue with a warning; it must be aware that it might conflict at schema level again, as the TD is out-of-date.
  2. This is clearly a problem of the TD "designer"; I also only see mitigation through validation tools to help TD authors not to produce faulty TDs
  3. As I discussed in my long text above, these are tow different cases:
    • The Thing (server) does not require an Accept header/option, because it only offers a single content type or it uses different resources for different content types --> no binding term given
    • The Thing (server) does require the Accept header/option --> it is clearly documented by including it in the TD

There are then still some resulting issues, where optimizations such as combining ops does not work anymore, as for instance a GET request for read requires only the Accept header, while a PUT requires the Content-Type header to be set. However, this optimization has always been intended for simple cases were it works, and not to add more complex rules for complex cases...

egekorkan commented 8 months ago

As shown in #1246, this should be handled one way or another. There could be multiple unknown contentTypes in a TD and the client should send the ones it can understand or the one indicated in formIndex. The things should still accept */* and no accept header and they can return anything.

egekorkan commented 6 months ago

@relu91 will summarize the discussion but from today's discussion in the committer call:

JKRhb commented 6 months ago
  • We should check the CoAP spec and adapt for the binding-coap

RFC 7252 states the following the regarding the Accept option:

[...] If no Accept option is given, the client does not express a preference (thus no default value is assumed). The client prefers the representation returned by the server to be in the Content-Format indicated. The server returns the preferred Content-Format if available. If the preferred Content-Format cannot be returned, then a 4.06 "Not Acceptable" MUST be sent as a response, unless another error code takes precedence for this response.

So the case that a server replies with a different Content-Format (i.e., a combination of content type and content coding) is not really supposed to happen. If I am not mistaken, it is not specified how the client should behave in this case if it actually does happen. Due to the MUST, though, the client could even send a "Reset message" to the server if it receives a non-error response with a different Content-Format (indicating that the response cannot be properly processed) and throw an error. Then again, a TD consumer could still try to use the returned value if it does support the Content-Format and only send a Reset message when it doesn't. This probably depends on how strict we want a consumer to be when it comes to handling this.

(@mkovatsc @ektrah Please correct me if I got anything wrong here.)

relu91 commented 6 months ago

So I re-read the whole thread, the linked issue, and a couple of TD sections (in particular this one). In summary, I confirm that the action points described above are confirmed:

  • Consumer should send the content type of the form in the accept header
  • If a thing replies with a different content type, the Consumer accommodates (still a warning is logged) if it can decode the payload.

Practically, we are saying that for every "read" operation of an HTTP binding, we are implicitly declaring that the Accept header equals to contentType, in other words:

{
  # a HTTP form
  "href": "http://example.com/properties/temperature",
   "op": "readproperty",
   "contentType": "application/json"
   # implicit or "default" information
   "htv:headers": [{
      "headerName": "Accept",
      "headerValue": "application/json"
   }]
}

Points .2 and .3 of my previous post were asking about how to deal with manual overrides of the "Accept" headers. So basically, we have the default, but what happens if the user decides to declare that header himself:

{
  # a HTTP form
  "href": "http://example.com/properties/temperature",
   "op": "readproperty",
   "contentType": "application/json"
   "htv:headers": [{
      "headerName": "Accept",
      "headerValue": "application/*"
   }]
}

or overrides but it is wrong:

{
  # a HTTP form
  "href": "http://example.com/properties/temperature",
   "op": "readproperty",
   "contentType": "application/json"
   "htv:headers": [{
      "headerName": "Accept",
      "headerValue": "audio/aac"
   }]
}

From the answers above and my personal preference, I would override the node-wot default behaviour and use what the user gives. If something is off we just log the warning and do our best to do the request and read the data. Of course. as pointed in the action points above, if we can't decode the payload we stop and throw an error.

relu91 commented 6 months ago

Call 03/05/2024 We should not outsmart the user in handling headers, if the headers are defined we need to follow his guidance.