Re: Small patch review for html begin/end inclusion files

[Ingo Schwarze <schwarze@usta.de>]

Salut Jean-Yves,

Jean-Yves Migeon wrote on Fri, Oct 23, 2015 at 09:58:26PM +0200:

> I am about to migrate man.netbsd.org to man.cgi
> to serve NetBSD's man pages

Nice to hear that!

> and I'd like to propose for review the attached patch that allows
> handling begin/end HTML tweaks without relying on CUSTOMIZE_BEGIN.
> 
> Reason behind is that our HTML templates can evolve at any time, and
> re-compiling is tedious just for manipulating the HTML code. Also HTML
> navigation bars and equivalents require the use of opening (resp.
> closing) div's before (resp. after) man.cgi main output, so _END
> directive (or equivalent) are needed.

After pondering it a bit, i see the point.
Fortunately, these requirements do not cause much complication
of the code or of the user interface, so i could simply integrate
a solution for them into the bsd.lv repo.

> The attached patch implements the following:
> - a CUSTOMIZE_END directive, counterpart to CUSTOMIZE_BEGIN, but inserts
> the code just before the closing </BODY> anchor;

When we support "header.html" and "footer.html", i don't see the
point of continuing support for CUSTOMIZE_BEGIN.  We don't need
two solutions for the same purpose.  So i deleted support for
CUSTOMIZE_BEGIN completely.

> - code to "internalize" HTML code directly from two files, called
> "begin.html" and "end.html" located in the same dir as manpath.conf.
> Purpose here is to avoid CUSTOMIZE_BEGIN/_END hardcoding at compile-time
> and put HTML code in an easier way to manipulate than multiline C strings.

I preferred calling the files "header.html" and "footer.html"; that's
just another colour for the bikeshed, but since this is not widely
used yet, i think such trivial changes won't cause issues for anyone.

> I would like to see such a feature imported in man.cgi (or another kind
> of templating engine if it exists). I am unsure about the way the
> begin/end.html handling should happen though:
> - a compile-time option, eventually combined with CUSTOMIZE_* macros;
> - having a special treatment regarding ENOENT during fopen(2) to not
> warn about their absence;
> - some other approach... ?

I think none of that is needed.  Let's just always try to read the
files, and if they are not there, ignore that.  I don't see why any
error handling should be done.  Even is the files are unreadable or
there is some other configuration error with respect to them, the
end-user is best served by ignoring the problem and seeing the
content of the manual.  The server admin can easily notice any
problems by seeing that the header and/or footer are not displayed.

So, i committed the following version.

Thanks for your suggestion!
  Ingo


Log Message:
-----------
Use include files "header.html" and "footer.html" rather than a
compiled-in string.  This is not a security risk, we read the file
manpath.conf from the same directory, anyway.  No error handling
is needed; even if the files are absent, that's not an error.

This is more flexible without causing complication of the code or
the user interface.  It helps the upcoming revamp of the online
manual pages on man.NetBSD.org.

Based on an idea by Jean-Yves Migeon <jeanyves dot migeon at free dot fr>,
but implemented in a much simpler way.

Modified Files:
--------------
    mdocml:
        cgi.c
        cgi.h.example
        man.cgi.8

Revision Data
-------------
Index: cgi.c
===================================================================
RCS file: /home/cvs/mdocml/mdocml/cgi.c,v
retrieving revision 1.113
retrieving revision 1.114
diff -Lcgi.c -Lcgi.c -u -p -r1.113 -r1.114
--- cgi.c
+++ cgi.c
@@ -77,6 +77,7 @@ static	void		 pg_searchres(const struct 
 static	void		 pg_show(struct req *, const char *);
 static	void		 resp_begin_html(int, const char *);
 static	void		 resp_begin_http(int, const char *);
+static	void		 resp_copy(const char *);
 static	void		 resp_end_html(void);
 static	void		 resp_searchform(const struct req *);
 static	void		 resp_show(const struct req *, const char *);
@@ -368,6 +369,20 @@ resp_begin_http(int code, const char *ms
 }
 
 static void
+resp_copy(const char *filename)
+{
+	char	 buf[4096];
+	ssize_t	 sz;
+	int	 fd;
+
+	if ((fd = open(filename, O_RDONLY)) != -1) {
+		fflush(stdout);
+		while ((sz = read(fd, buf, sizeof(buf))) > 0)
+			write(STDOUT_FILENO, buf, sz);
+	}
+}
+
+static void
 resp_begin_html(int code, const char *msg)
 {
 
@@ -384,12 +399,16 @@ resp_begin_html(int code, const char *ms
 	       "<BODY>\n"
 	       "<!-- Begin page content. //-->\n",
 	       CSS_DIR, CUSTOMIZE_TITLE);
+
+	resp_copy(MAN_DIR "/header.html");
 }
 
 static void
 resp_end_html(void)
 {
 
+	resp_copy(MAN_DIR "/footer.html");
+
 	puts("</BODY>\n"
 	     "</HTML>");
 }
@@ -399,7 +418,6 @@ resp_searchform(const struct req *req)
 {
 	int		 i;
 
-	puts(CUSTOMIZE_BEGIN);
 	puts("<!-- Begin search form. //-->");
 	printf("<DIV ID=\"mancgi\">\n"
 	       "<FORM ACTION=\"%s\" METHOD=\"get\">\n"
Index: cgi.h.example
===================================================================
RCS file: /home/cvs/mdocml/mdocml/cgi.h.example,v
retrieving revision 1.3
retrieving revision 1.4
diff -Lcgi.h.example -Lcgi.h.example -u -p -r1.3 -r1.4
--- cgi.h.example
+++ cgi.h.example
@@ -4,6 +4,4 @@
 #define	MAN_DIR "/var/www/man"
 #define	CSS_DIR ""
 #define	CUSTOMIZE_TITLE "Manual pages with mandoc"
-#define	CUSTOMIZE_BEGIN "<H2>\nManual pages with " \
-	"<A HREF=\"http://mdocml.bsd.lv/\">mandoc</A>\n</H2>"
 #define	COMPAT_OLDURI Yes
Index: man.cgi.8
===================================================================
RCS file: /home/cvs/mdocml/mdocml/man.cgi.8,v
retrieving revision 1.12
retrieving revision 1.13
diff -Lman.cgi.8 -Lman.cgi.8 -u -p -r1.12 -r1.13
--- man.cgi.8
+++ man.cgi.8
@@ -190,14 +190,8 @@ and to be specified without a trailing s
 When not specified, the CSS files
 are assumed to be in the document root.
 This is used in generated HTML code.
-.It Ev CUSTOMIZE_BEGIN
-A HTML string to be inserted right after opening the
-.Aq BODY
-element.
 .It Ev CUSTOMIZE_TITLE
-An ASCII string to be used for the HTML
-.Aq TITLE
-element.
+An ASCII string to be used for the HTML <TITLE> element.
 .It Ev HTTP_HOST
 The FQDN of the (possibly virtual) host the HTTP server is running on.
 This is used for
@@ -374,6 +368,12 @@ or any character not contained in the
 .Sx Restricted character set ,
 .Nm
 reports an internal server error and exits without doing anything.
+.It Pa /man/header.html
+An optional file containing static HTML code to be inserted right
+after opening the <BODY> element.
+.It Pa /man/footer.html
+An optional file containing static HTML code to be inserted right
+before closing the <BODY> element.
 .It Pa /man/OpenBSD-current/man1/mandoc.1
 An example
 .Xr mdoc 7
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv