[haiku-commits] Re: haiku: hrev50368 - src/kits/network/libnetapi headers/os/net

  • From: Hamish Morrison <hamishm53@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Wed, 22 Jun 2016 23:56:59 +0100

Hi Mark,

On 22/06/2016 22:37, Mark Hellegers wrote:

I'm a bit surprised there is this much discussion on this commit as the 
patch was basically unchanged on Trac for weeks with hardly any comment 
at all.

My apologies - I didn't read the patch too closely on trac.

But as to the concerns about the patch:
The hostname is used in the application layer, but that does not make 
it an application concern. It is a network address and I see no problem 
with storing it at the internet layer in BNetworkAddress. It already 
gets it anyway, we just didn't store it before. There was even already 
a function to retrieve the hostname in BNetworkAddres. It simply was 
not implemented.

It's an application concern because the application knows how it should
be used. It should be the application's decision whether to send along
the hostname for server name indication. It shouldn't depend on whether
I happened to construct the BNetworkAddress with a string hostname or
not. Indeed it's perfectly possible to construct a BNetworkAddress with
an IP address. With this interface, it's impossible to use TLS SNI with
a BNetworkAddress constructed this way.

The hostname, and how this plays a part in TLS server name indication,
is the concern of the higher level protocol, TLS. The network layer
doesn't care about what hostname the network address sprung from; all it
cares about is the IP address to connect to.

What was a fairly simple wrapper around a socket address has now become
a socket address plus a hostname which may or may not be present
depending on how it was constructed. It violates the single
responsibility principle.

Also, it seems awkward to force the caller to call an extra function 
simply to set the hostname while BNetworkAddress seems perfectly 
capable of holding that information. I would think that encourages 
buggy code by applications forgetting to set it, while it is now 
automatic as long as BNetworkAddress is called with a hostname.

The converse of this is that it's now forced upon the application
whether it wants it or not. An application now has to jump through hoops
to disable SNI or if they want to use it with a socket address they've
resolved by other means.

Cheers,
Hamish

Other related posts: