From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: diff httpd: avoid misleading syslog warnings
To: tech@openbsd.org
Date: Tue, 13 Feb 2024 14:46:36 +0100

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);