Re: [PATCH] xgethostname: handle long hostnames

["René Scharfe" <l.s.r@web.de>]

Am 13.04.2017 um 21:23 schrieb David Turner:
> If the full hostname doesn't fit in the buffer supplied to
> gethostname, POSIX does not specify whether the buffer will be
> null-terminated, so to be safe, we should do it ourselves.  Introduce
> new function, xgethostname, which ensures that there is always a \0
> at the end of the buffer.
> 
> Signed-off-by: David Turner <dturner@twosigma.com>
> ---

> diff --git a/wrapper.c b/wrapper.c
> index 0542fc7582..d837417709 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -655,3 +655,16 @@ void sleep_millisec(int millisec)
>   {
>   	poll(NULL, 0, millisec);
>   }
> +
> +int xgethostname(char *buf, size_t len)
> +{
> +	/*
> +	 * If the full hostname doesn't fit in buf, POSIX does not
> +	 * specify whether the buffer will be null-terminated, so to
> +	 * be safe, do it ourselves.
> +	 */
> +	int ret = gethostname(buf, len);
> +	if (!ret)
> +		buf[len - 1] = 0;
> +	return ret;
> +}

Silent truncation is not ideal, no matter if it's done by the wrapper or
the original function.  It would be better to use a properly sized
buffer.

POSIX requires hostnames to have a maximum length of HOST_NAME_MAX.  So
how about just adding an assert to make sure len is big enough?  Or
evaluate the condition at compile time with BUILD_ASSERT_OR_ZERO?

Downside: Not all platforms define HOST_NAME_MAX.  daemon.c uses 256 as
a fallback.  On Windows a buffer size of 256 is documented to suffice
in all cases [1].  The Linux manpage [2] mentions a hostname length
limit of 255 (plus NUL) as well, even though HOST_NAME_MAX is 64 there.

Another possibility: Die (or at least warn) if the buffer doesn't
contain a NUL byte after calling gethostname().  That only works for
platforms that don't NUL-terminate on truncation, though, so silent
truncation would still go unnoticed.

Anyway, the buffer in builtin/gc.c with its 128 bytes seems to be too
short; the others are at least 256 bytes long.  Replacing the magic
buffer size number with HOST_NAME_MAX + 1 might be a good idea (after
moving the fallback definition to git-compat-util.h).

René


[1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms738527(v=vs.85).aspx
[2] http://man7.org/linux/man-pages/man2/gethostname.2.html