[Captive-portals] Review of draft-ietf-capport-rfc7710bis

[Lorenzo Colitti <[e-mail]>]

I see no major issues and think this draft is ready to advance. A few nits
follow:

1. I think it would be good if the document said that the the API URL
SHOULD be the same for all clients, and that any per-client behaviour
should instead be implemented by making the content of the URL dynamically
generated, if necessary. In the same spirit, this text probably needs an
upper-case SHOULD:

====
   The captive portal operator should ensure that the
   URIs handed out are equivalent to reduce the chance of operational
   problems.  The maximum length of the URI that can be carried in IPv4
   DHCP is 255 bytes, so URIs longer than 255 bytes should not be used
   in IPv6 DHCP or IPv6 RA.
=====


2. I'm surprised that the following text is present. It seems like we
should disallow IP literals for compatibility with IPv6. But perhaps SHOULD
is enough here.

   The URI SHOULD NOT contain an IP address literal.  The URI parameter
   is not null terminated.



3. The section that documents the link relation type should mention what
should happen if the portal is already open. Should the captive portal add
this header to probe responses even if the portal is already open? if it
does not, there is no way for a device to learn the API URL if it connects
to a portal, logs in, disconnects, and then reconnects, because when it
reconnects the portal will be open.


4. In section 4, is it even worth specifying the precedence order? What if
we replaced the entirety of section 4 with:

   A device may learn about Captive Portal API URIs through more than
   one of (or indeed all of) the above options.  It is a network
   configuration error if the learned URIs are not all identical.

   If the URIs learned are not identical, clients MAY use one of more of the
   URIs without distinction.

That would IMO decrease the chances of such a configuration error being
made. :-)