Subject: [core] Short review of SenML fetch/patch

From: Christian Amsüss <[email-masked]>
Date: Thu, 27 Sep 2018 10:59:48 +0200

Hello authors,

as requested during yesterday's interim meeting, I've had a look at
senml-fetch again:

Fetch
=====

Fetching by time
----------------

Being able to pick aparticular timestamp is an oddly specific
application; my impression of general SenML data sets is that it will be
hard to predict which point in time should have a datum if that record
is not already known. Unless there are good (and argued for) use cases,
I suggest that the topic of fetching time series points be explored
comprehensively in a separate document, possibly hooking into
draft-bormann-t2trg-stp.

Using SenML to fetch
--------------------

SenML requires to have a value (or sum) on every record, the payload of
the fetch request therefore can't use any of the defined SenML mime
types but would need to specify their own. I'm unsure on whether adding
a field like "fetch_":1 to the first record (a mandatory-to-decode
custom field) would be sufficient to work around that constraint.

Otherwise, it might be an option to update RFC8428 to say that some of
those limitations (also the later "v":null) may be overridden in
applications like this where it is made sure (eg. by never using those
freedoms in Content or PUT payloads) that no generic SenML processor
ever gets its hands on the document.

The payload sent in as a FETCH payload is not actually sensor data, it's
a list of names (or, provided they're viewed as URIs, which I recommend)
links. We already have a way to transmit a list of links in CoRE
(actually 3 with links-json). I don't object to introducing another
(given the above, might be called application/links+senml+{cbor,json}?)
especially to reduce parsing complexity, but do suggest that FETCHing
SenML data be done with a list of identifiers no matter how they are
transported, and the recommended way can well be a SenML-names-only
payload (but would not preclude others).

Patch
=====

"value field with value null": There has been some backpressure against
having variable-type fields in SenML back when its JSON/CBOR structure
was last changed; having a "v":null in there could be problematic (esp.
as its JSON type is fixed to Number in Table 2 of RFC8428). I suggest
having a explicit "delete value" field, eg. "vdel":true.

How would base values work with patch? I figure that it's always the
resolved record that matter, but that should be explicit. In connection
with a "vdel" (or "v":null if it turns out it's OK for it to stay like
that), it might make sense to allow that too (where a "v":n would
override "vdel" or "v":null from an offset of zero) to allow for
efficient batch removal of values.

Security considerations
=======================

"potentially allow retrieving or changing many resources at once" makes
it sound like FETCH and PATCH could reach out to different resources.
They don't (conceptually), they only manipulate what's inside the target
resource (which typically is an aggregation of other resources, as in a
core-interfaces batch). Suggested replacement:

   In FETCH and iPATCH requests, the client can pass arbitrary names to
   the target resource for manipulation. The resource implementer must
   take care to only allow access to names that are actually part of (or
   accessible through) the target resource.
   
   If the client is not allowed to do a GET or PUT on the full target
   resource (and thus all the names accessible through it), access
   control rules must be evaluated for each record in the pack.

(It's fine if someone goes ahead and defines a /senml resource on their
device that is a gate to "everything and anything that can be addressed
in SenML", but that shouldn't be the default assumption).

Best regards
Christian

-- 
You don't become great by trying to be great. You become great by
wanting to do something, and then doing it so hard that you become great
in the process.
  -- Marie Curie (as quoted by Randall Munroe)

---

From: Ari Keränen <[email-masked]>
Date: Thu, 27 Sep 2018 18:07:28 +0000

Thank you for the review Christian!

See inline.

> On 27 Sep 2018, at 11.59, Christian Amsüss <[email-masked]> wrote:
> 
> Hello authors,
> 

The time-stamp use case I had in mind was where the record is already known, so it's mainly useful for PATCHing, but didn't see issue having it for FETCH too (say, you know time out of band). Otherwise we have no mechanism to tell apart two records from the same source.

> Using SenML to fetch

Good point. Maybe updating 8428 with the exception that restrictions on value don't apply for FETCH/PATCH is appropriate.

> payload (but would not preclude others).

The payload is a list of names without URI semantics (e.g., may not have scheme at all, have different concatenation semantics, etc.) and timestamps, so I think it's quite different from the other link formats.

> having a explicit "delete value" field, eg. "vdel":true.

The "v":null approach made this nice and clean, but you do have a valid point to consider here. Will have a closer look at this.

> efficient batch removal of values.

Yes, resolved records are that matter (see end of section 3.1). I'm not quite sure what you suggest allowing though?

> Security considerations

Good point. The replacement looks good to me.

Thanks,
Ari