From: Florian Obser <florian@openbsd.org>
Subject: Re: diff httpd: avoid misleading syslog warnings
To: tech@openbsd.org
Date: Tue, 13 Feb 2024 14:49:03 +0100

OK

On 2024-02-13 14:46 +01, Claudio Jeker <cjeker@diehard.n-r-g.com> wrote:
> On Tue, Feb 13, 2024 at 02:35:47PM +0100, Florian Obser wrote:
>> On 2024-02-13 13:17 +01, "Carsten Reith" <carsten.reith@t-online.de> wrote:
>> > Dear all,
>> >
>> > Currently httpd writes a misleading error (log_warn)  message to syslog in the following
>> > scenario:
>> >
>> > The user defined the errdoc directive in the httpd.conf.
>> > The user created custom error pages for some http error codes, let's say
>> > 404.html.
>> > The user created a custom generic error page err.html.
>> >
>> > Now, if an error occurs _without_ a specific custom error page, httpd
>> > writes an error message to syslog:
>> >
>> > serv1 httpd[23368]: read_errdoc: open: No such file or directory
>> >
>> > Then it delivers the custom error page correctly. (or the builtin one,
>> > in case no custom err.html exists, resulting in another syslog warning.)
>> >
>> > The following diff avoids this confusing error message (it is
>> > misleading as the specific error page is not supposed to exist from a
>> > user point of view.)
>> >
>> > cheers,
>> >
>> > Carsten
>> >
>> > Index: server_http.c
>> > ===================================================================
>> > RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
>> > retrieving revision 1.153
>> > diff -u -p -u -p -r1.153 server_http.c
>> > --- server_http.c	21 Sep 2022 05:55:18 -0000	1.153
>> > +++ server_http.c	13 Feb 2024 12:08:49 -0000
>> > @@ -1766,6 +1766,8 @@ read_errdoc(const char *root, const char
>> >  
>> >  	if (asprintf(&path, "%s/%s.html", root, file) == -1)
>> >  		fatal("asprintf");
>> > +	if (access(path, R_OK) == -1)
>> > +		return (NULL);
>> 
>> what do you do if err.html gets deleted between access(2) and open(2)?
>> 
>> >  	if ((fd = open(path, O_RDONLY)) == -1) {
>> >  		free(path);
>> >  		log_warn("%s: open", __func__);
>> >
>
> I guess it would be better to only call log_warn() if the errno is not
> ENOENT. That should be enough to silence the common case.
>
> -- 
> :wq Claudio
>
> Index: server_http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> diff -u -p -r1.153 server_http.c
> --- server_http.c	21 Sep 2022 05:55:18 -0000	1.153
> +++ server_http.c	13 Feb 2024 13:45:26 -0000
> @@ -1762,13 +1762,14 @@ read_errdoc(const char *root, const char
>  	struct stat	 sb;
>  	char		*path;
>  	int		 fd;
> -	char		*ret = NULL;
> +	char		*ret;
>  
>  	if (asprintf(&path, "%s/%s.html", root, file) == -1)
>  		fatal("asprintf");
>  	if ((fd = open(path, O_RDONLY)) == -1) {
>  		free(path);
> -		log_warn("%s: open", __func__);
> +		if (errno != ENOENT)
> +			log_warn("%s: open", __func__);
>  		return (NULL);
>  	}
>  	free(path);
> @@ -1788,8 +1789,7 @@ read_errdoc(const char *root, const char
>  		log_warn("%s: read", __func__);
>  		close(fd);
>  		free(ret);
> -		ret = NULL;
> -		return (ret);
> +		return (NULL);
>  	}
>  	close(fd);
>  
>

-- 
In my defence, I have been left unsupervised.