Re: [Captive-portals] AD review of draft-ietf-capport-api-06

[Barry Leiba <[e-mail]>]

Thanks for the updates, Tommy... I have requested last call on -07,
and that will come out as soon as the Secretariat does their magic.

Barry

On Mon, Apr 27, 2020 at 1:55 PM Tommy Pauly <[e-mail]> wrote:
>
> Hi Barry,
>
> Thanks for the review! Update made here:
>
>  https://github.com/capport-wg/api/commit/27cd22fd8a8db696efb9772acd6c05d24a86c81d
>
> This has been posted as draft-ietf-capport-api-07: https://tools.ietf.org/html/draft-ietf-capport-api-07
>
> On Apr 23, 2020, at 10:03 PM, Barry Leiba <[e-mail]> wrote:
>
> Thanks for a nice document.  Here’s my AD review prior to last call.  For a few reasons, I think we need a revised I-D before going to last call.
>
> — Section 2 —
>
>    *  Captive Portal API Server: The server exposing the API's defined
>
> Nit: no apostrophe should be in “APIs” (it’s not possessive).
>
>
> Fixed
>
>
> This section also needs BCP 14 boilerplate (and the document needs normative references to RFCs 2119 and 8174), as Sections 4 and 5 use BCP 14 key words.
>
>
> Added
>
>
> — Section 3 —
> I think 7710bis is an informative reference, not a normative one.
>
>
> Changed to informative.
>
>
> — Section 4.1 —
>
>    valid certificate for the hostname in the URI (such as
>    "example..com").
>
> It’s a small point, and not wrong as written, but as there is an example already given right above at the end of Section 4, I suggest referring to it:
>
> NEW
>    valid certificate for the hostname in the URI
>    ("example.org", in the example above).
> END
>
>
> Changed as suggested.
>
>
> — Section 5 —
>
> In the bullet about seconds-remaining:
>
>       The API server SHOULD include this value if the client is
>       not captive (i.e. captive=false) and the client session is time-
>       limited, and SHOULD omit this value for captive clients (i.e.
>       captive=true).
>
> What about captive=false and the client is not time-limited?  I think you want, “and SHOULD omit this value value for captive clients (i.e. captive=true) or when the client sessiin is not time-limited.”  Or, simpler and better, “and SHOULD omit this value otherwise.”
>
> And similarly for the next bullet about bytes-remaining.
>
>
> Changed for both cases.
>
>
> — Section 6 —
> The example violates the SHOULD in Section 5, “seconds-remaining”.  That’s not a good idea.  It might be good to have two examples, one with captive=true and one with captive=false.
>
>
> Split the server response example into two.
>
>
>    Upon receiving this information the client will provide this
>    information to the user so that they may navigate the web portal
>
> The client isn’t really going to provide the information to the user, is it?  It will provide the information to another application, or will use the information to allow the user to navigate the portal.  Can you rephrase this so it doesn’t sound as if the client will say, “Here’s the portal URI; go wild.” ?
>
>
> The sentence now reads: "Upon receiving this information the client will use this information direct the user to the the web portal (as specified by the user-portal-url value) to enable access to the external network."
>
>
> — Section 8.1 —
> Please double-check the registration template for media types in RFC 6838 and make sure this complies.
>
>
> Updated for the missing "Fragment identifier considerations” field.
>
>
> — Section 8.2 —
>
>    New assignments for Captive Portal API Keys Registry will be
>    administered by IANA through Expert Review [RFC8126].  The Designated
>    Expert is expected to validate the existence of documentation
>    describing new keys in a permanent publicly available specification.
>
> How, then, is this not Specification Required?
>
>
> Good point =) Updated to Specification Required, as that is what the text explains.
>
> Best,
> Tommy
>
>
> —
> Barry
>
> _______________________________________________
> Captive-portals mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/captive-portals
>
>