Re: [core] Review on draft-ietf-core-coap-pubsub-02

[Michael Koster <[e-mail]>]

Hi Jim,

Thank you for taking the time to evaluate this draft. We have discussed each point and have included the discussion and resolution inline in this response.

> On Jul 10, 2017, at 11:57 AM, Jim Schaad <[e-mail]> wrote:
> 
> Status:  This document is not yet ready for a WGLC in my opinion. 
> 
> I have copied forward some comments from past reviews where I have not seem
> them addressed or do not believe that they are sufficiently  dealt with.
> 
> 
> 
> * Section 3.4 - s/are hosted at the broker and all clients using the broker
> share/are hosted by a broker and all clients using that broker share/ - a)
> you can have multiple brokers and b) the name spaces in that case are going
> to be distinct and potentially overlapping in name w/different values.

Yes, updated
> 
> * Section 3.4 - "zero or more stored representations with a content-format"
> I think this is supposed to be  "zero or more values - one content-format".
> But the first time I read this, I read this as" zero or more representations
> - each representation has a content format - may be zero or one (?) value".
> I always worry about what is "current" not what is "history" and assume that
> observe is correctly implemented.  Note this does go back to the question of
> "is this a queue of values" that Hannes has raised in the past.
> 
We think stored representations is correct, as a broker only stores representations and is not aware of values.
We are restricting it to most recently published representation, and should take care that this is clear.

For queueing and reliable notification, we consider a future extension based on subscriber-managed queues. 
See github issues: 
https://github.com/core-wg/pubsub/issues/5 <https://github.com/core-wg/pubsub/issues/5>

> * Section 4.2 - cut and paste error for topic and q as template variables.
> I believe that q should be removed from the template as it makes no sense.
> 
Yes, corrected.

> * Section 4.2 - Is the following legal for payload of a create
> "<mainTopic/subTopic>;ct=50"  - this is a legal link but may not be legal
> for creation of a topic.  
> 
Yes, we will clarify what is and is not legal for create. Probably only one topic tree level at a time.

> * Section 4.2 - Is the following legal for the payload of a create
> "<coap://localhost/ps/maintopic/subtopic>;ct=50" - The text current says the
> target of the link is a "URI formatted string" rather than a fragment.
> 
Same as above.WIll clarify in the next update.

> * Section 4.2 - I cannot tell if the requirement for returning 4.04 (not
> found) is new in this document or should be part of RFC 6690 or not.  I
> cannot seem to find this requirement in that document.  Are you sure you
> want to do this rather than just return an empty content?  Should there be
> an errata on RFC 6690?
> 
RFC6690 should not specify how REST APIs are expected to expose links. It should focus on the format and just provide examples. 4.04 is returned when the topic has been removed. Subscribe to non-existant topics may be covered in the subscriber queue extension described above.

> * Section 4.2 - There is an implicit statement in section 4.3 that a single
> content type is expected.  However, there is no explicit statement here that
> a request with multiple content types is forbidden.  Is there really a
> requirement that a single content type be required when creating a topic?
> What happens if no 'ct' is specified?
> 
Candidate text:
A topic creator includes one or more content format option values in the create payload. If the broker does not support all of the indicated formats for both publish and subscribe, it (SHALL) returns an error response.

If a broker allows a topic to be created with more than one content-format, it SHALL support an arbitrary mix of the supported content formats in publish and subscribe operations.

There is no default content format. If no ct is specified, the broker SHALL reject the operation with an error code 4.15 "unsupported content format" 

> * Section 4.3 - Is a server supposed to accept a publish operation to a
> non-existent location with a POST method? 
> 
Create-on-publish must use PUT. The link is constructed from the request parameters. WIll clarify in the next update.

> * Section 4.3 - Is there any reason to accept publish via POST and not PUT?
> Or the other way around?
> 
PUT is required for create-on-publish

POST is prefereble when the topic already exists, but PUT may be used also. 

Generically, PUT is defined as replace the stored representation with the supplied representation, may create if the server supports (i.e. a PUT to a non-existent location may return 2.01 Created or 4.04 Not Found)

> * Section 4.3 - If no Max-Age is on the publish request, should the default
> value be used or should an infinite value (i.e. absent) be used.  If a
> Max-Age was supplied at the time the subject was created, does that change
> things?  If a publish is done with a Max-Age greater than that provided at
> creation, what should happen.  Which wins?
> 
More explanatory text is perhaps needed. There are two Max-Age contexts, one is for the topic and one is for the value.

If no Max-Age is supplied, the topic should be permanent, modulo the server's abiity to store topics.

Max-Age on publish should default to permenent also.

> * Section 4.3 - Copy and paste error on the topic and q template variables.
> Does 'q' even make sense here?
> 
> * Section 4.3 - Text dealing with propagating publish events up the tree to
> keep nodes alive is missing.
> 
Yes, this is the intended behavior and will be described in the next update.

> * Section 4.4 - Copy and paste error on the topic and q template variables.
> Does 'q' even make sense here?
> 
Yes, corrected in the next update.

> * Section 4.4 - Is it permitted that a broker remove a subscription due to
> lack of ACK for a CON w/o sending the "unsubscribed" message?  The document
> does not seem to allow that.
> 
If a broker removes a topic, it is allowed to do so without informing the subscribers. We use a SHOULD statement to indicate that. 

If a client fails to ACK an observe response when the broker uses CON, the broker should not try to send a final message. We will clarify this. RFC7641 section 3.5 doesn't seem to explain this either:

   An acknowledgement message signals to the server that the client is
   alive and interested in receiving further notifications; if the
   server does not receive an acknowledgement in reply to a confirmable
   notification, it will assume that the client is no longer interested
   and will eventually remove the associated entry from the list of
   observers (Section 4.5 <https://tools.ietf.org/html/rfc7641#section-4.5>).

> * Section 4.5 - Copy and paste error on the topic and q template variables.
> Does 'q' even make sense here?
> 
Yes, will correct in the next update.

> * Section 4.6 - Copy and paste error on the topic and q template variables.
> Does 'q' even make sense here?
> 
Yes. Will correct.

> * Section 4.7 - Is remove recursive?   
> 
Remove is inclusive of sub-topics, or recursive from the bottom of the tree. WIll add text to clarify.

> * Section 4.7 - What would the correct content be to send an unsubscribe
> notification on a remove operation?  '2.07 no content' or '2.02' deleted or
> '4.04' not found?
> 
2.02 Deleted would be the response to the Deleteing client, 4.04 woud be sent to observing clients, as per standard CoAP Observe. Will make sure this is clear in the next update.

> * Section 8 - If enforcing access policies is of importance, how are they
> set?
> 
The reference is toward an ACL type system that may be out of the scope of this draft. More could be said about what to do and how to do it. Will address this after some more discussion.

> * Section 8 - It may be that '4.01 Unauthorized' is a better response than
> '4.04 Not Found' in some cases.  Doing this does not allow for probing of
> what topics exist when one does not have access to the directory of topics.
> 
Good point. This may be added to security considerations after discussion. It would be better to have explicit ACLs that deny access to the resources and requre returning 4.01 in these cases.
> 
> * Consider the following scenario
>  - Create node foo - ct=40, Max-Age=10
>  - Create node foo/bar - ct=0, Max-Age=20
>  - Publish to foo/bar @ time=5
>  When you get to time=15, should the node foo be deleted or does the
> max-age of foo/bar change that answer?
> 
foo expires and inclusively removes foo/bar at t=15

> * Currently an application can use CON to ensure that a server will get a
> message in the long run.  The fact that this is no doable here should be
> briefly discussed as an client cannot know that a server has or has not
> received a message.
> 
Yes, end-to-end protocol does not exist in pubsub. We have stayed away from describing transfer layer operations in this draft, but it seems like a good idea to point out the considerations in an informative way here as in the issue above about observe termination. 
> 
> Minor:
> 
> * Section 3.4 -  s$"EP-33543/sen$"/EP-35543/sen$ - either that or remove the
> leading slash for sensors
> 
Yes, appears to be a typo.
> 
> Philosophy problem:
> 
> I am not happy with the way that 2119 language is sprinkled throughout this
> document.  That is not a statement that you need to change it, but it is a
> request that you revisit and look at what is there.  
> * MUST and SHOULD statements need to be able to have some type of test
> written from them.  If you cannot write a test, then using the key words is
> probably incorrect.
> * SHOULD and MAY statements ought to have some type of language about when
> the action under consideration would not be done.  
> * MAY statements are almost always a waste of time and should be written in
> a different manner. 
>    *  Consider the first sentence in section 4.3.  Is there a reason why
> this is not changed to "This section is optional to implement." and leave it
> at that?  
>    *  Consider the second sentence in section 4.3.  It implies that there
> is a third method that the client can use to publish, what is that method?
> I don't currently know of one.  This saying "A client uses the POST or PUT
> method to publish ..." seems to be an adequate statement w/o the MAY.  
> * Once upon a time, there was a requirement to extract and test all of the
> MUSTs in a document and a strong suggestion to test all of the SHOULDs in a
> document.  Too many make things hard to see if you have adequate coverage.
> Too few and you might not have interop working correctly.  In many cases, a
> MUST can be expressed w/o the key work.  Consider the 2.04 sentence in
> section 4.3 paragraph 1 - It can be written as "The broker returns a "2.04
> Changed" response if the publish request is accepted".  This has the same
> information as the current statement but w/o the MUST.  The two following
> sentences should probably also be a MUST type statement so that a client
> knows what is going on rather than returning something strange.  (That can
> be countered in the case of '4.04 Not Found' to maybe be '4.01 Unauthorized'
> as a reasonable response to prevent knowledge of what topics exist.)
> 
Thanks for this advice. Will discuss and improve the normative language. Thinking about test cases is indeed a good way to crisp up the normative stuff, as it makes it painful to try to test things that don't need to be or shouldn't be prescriptive.