Small patch review for html begin/end inclusion files

Small patch review for html begin/end inclusion files

[Jean-Yves Migeon]

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]

Dear list,

I am about to migrate man.netbsd.org to man.cgi to serve NetBSD's man
pages 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.

The attached patch implements the following:
- a CUSTOMIZE_END directive, counterpart to CUSTOMIZE_BEGIN, but inserts
the code just before the closing </BODY> anchor;
- 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 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... ?

Suggestions welcome.

Thanks,

-- 
Jean-Yves Migeon

[-- Attachment #2: cgi.c.diff --]
[-- Type: text/plain, Size: 4121 bytes --]

:q— work/mdocml-1.13.3/cgi.c	2015-03-13 12:38:38.000000000 +0000
+++ cgi.c	2015-10-22 18:35:21.000000000 +0000
@@ -17,6 +17,7 @@
  */
 #include "config.h"
 
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/time.h>
 
@@ -52,6 +53,8 @@ struct	req {
 	struct query	  q;
 	char		**p; /* array of available manpaths */
 	size_t		  psz; /* number of available manpaths */
+	char		 *header; /* custom HTML header */
+	char		 *footer; /* custom HTML footer */
 };
 
 static	void		 catman(const struct req *, const char *);
@@ -63,6 +66,8 @@ static	void		 http_parse(struct req *, c
 static	void		 http_print(const char *);
 static	void		 http_putchar(char);
 static	void		 http_printquery(const struct req *, const char *);
+static	void		 internalize_file(char **, const char *);
+static	void		 htmlgen(struct req *);
 static	void		 pathgen(struct req *);
 static	void		 pg_error_badrequest(const char *);
 static	void		 pg_error_internal(void);
@@ -76,6 +81,7 @@ static	void		 resp_begin_html(int, const
 static	void		 resp_begin_http(int, const char *);
 static	void		 resp_end_html(void);
 static	void		 resp_searchform(const struct req *);
+static	void		 resp_footer(const struct req *);
 static	void		 resp_show(const struct req *, const char *);
 static	void		 set_query_attr(char **, char **);
 static	int		 validate_filename(const char *);
@@ -398,7 +404,10 @@ resp_searchform(const struct req *req)
 {
 	int		 i;
 
+	/* Write the customized HTML header */
 	puts(CUSTOMIZE_BEGIN);
+	if (req->header != NULL)
+		puts(req->header);
 	puts("<!-- Begin search form. //-->");
 	printf("<DIV ID=\"mancgi\">\n"
 	       "<FORM ACTION=\"%s\" METHOD=\"get\">\n"
@@ -490,6 +499,16 @@ resp_searchform(const struct req *req)
 	puts("<!-- End search form. //-->");
 }
 
+static void
+resp_footer(const struct req *req)
+{
+
+	/* Write the customized HTML footer */
+	if (req->footer != NULL)
+		puts(req->footer);
+	puts(CUSTOMIZE_END);
+}
+
 static int
 validate_urifrag(const char *frag)
 {
@@ -544,6 +563,7 @@ pg_index(const struct req *req)
 	       "manual explains the query syntax.\n"
 	       "</P>\n",
 	       scriptname, scriptname);
+	resp_footer(req);
 	resp_end_html();
 }
 
@@ -555,6 +575,7 @@ pg_noresult(const struct req *req, const
 	puts("<P>");
 	puts(msg);
 	puts("</P>");
+	resp_footer(req);
 	resp_end_html();
 }
 
@@ -678,6 +699,7 @@ pg_searchres(const struct req *req, stru
 		resp_show(req, r[iuse].file);
 	}
 
+	resp_footer(req);
 	resp_end_html();
 }
 
@@ -931,6 +953,7 @@ pg_show(struct req *req, const char *ful
 	resp_begin_html(200, NULL);
 	resp_searchform(req);
 	resp_show(req, file);
+	resp_footer(req);
 	resp_end_html();
 }
 
@@ -1060,6 +1083,7 @@ main(void)
 
 	memset(&req, 0, sizeof(struct req));
 	pathgen(&req);
+	htmlgen(&req);
 
 	/* Next parse out the query string. */
 
@@ -1102,10 +1126,68 @@ main(void)
 	for (i = 0; i < (int)req.psz; i++)
 		free(req.p[i]);
 	free(req.p);
+	free(req.header);
+	free(req.footer);
 	return(EXIT_SUCCESS);
 }
 
 /*
+ * Read and store content of a file located in MAN_DIR
+ */
+static void
+internalize_file(char **buf, const char *name)
+{
+	FILE	*fp;
+	struct stat st;
+
+	if (NULL == (fp = fopen(name, "r"))) {
+		fprintf(stderr, "%s/%s: %s\n",
+			MAN_DIR, name, strerror(errno));
+		goto error;
+	}
+
+	if (fstat(fileno(fp), &st) != 0) {
+		fprintf(stderr, "%s/%s: %s\n",
+			MAN_DIR, name, strerror(errno));
+		goto error;
+	}
+
+	*buf = mandoc_calloc(1, st.st_size + 1);
+	if (fread(*buf, sizeof(char), st.st_size, fp) < st.st_size) {
+		if (ferror(fp)) {
+			fprintf(stderr, "%s/%s: %s\n",
+				MAN_DIR, name, strerror(errno));
+			goto error;
+		}
+		if (feof(fp)) {
+			fprintf(stderr, "%s/%s: %s\n",
+				MAN_DIR, name, "premature EOF");
+			goto error;
+		}
+	}
+	fclose(fp);
+	return;
+
+error:
+	if (fp != NULL)
+		fclose(fp);
+	free(*buf);
+	*buf = NULL;
+	return;
+}
+
+/*
+ * Scan for customized HTML header and footer
+ */
+static void
+htmlgen(struct req *req)
+{
+
+	internalize_file(&req->header, "begin.html");
+	internalize_file(&req->footer, "end.html");
+}
+	
+/*
  * Scan for indexable paths.
  */
 static void

[-- Attachment #3: man.cgi.8.diff --]
[-- Type: text/plain, Size: 1392 bytes --]

--- work/mdocml-1.13.3/man.cgi.8	2015-03-13 12:38:38.000000000 +0000
+++ man.cgi.8	2015-10-22 19:20:42.000000000 +0000
@@ -14,7 +14,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: September 14 2014 $
+.Dd $Mdocdate: October 23 2015 $
 .Dt MAN.CGI 8
 .Os
 .Sh NAME
@@ -158,6 +158,26 @@ to create a
 .Xr mandoc.db 5
 database inside each manpath.
 .Pp
+You can optionally create a file
+.Pa /var/www/man/begin.html
+containing HTML code that will be inserted as-is right after
+the
+.Aq BODY
+element.
+It mimics the behaviour of the
+.Ev CUSTOMIZE_BEGIN
+compile-time option,
+except that the string is obtained through the file rather than
+specified during the compilation step.
+A similar file called
+.Pa /var/www/man/end.html
+will insert its HTML content right before the
+.Aq /BODY
+element,
+in the same way the
+.Ev CUSTOMIZE_END
+compile-time option would do.
+.Pp
 Configure your web server to execute CGI programs located in
 .Pa /cgi-bin .
 When using
@@ -191,6 +211,10 @@ This is used in generated HTML code.
 A HTML string to be inserted right after opening the
 .Aq BODY
 element.
+.It Ev CUSTOMIZE_END
+A HTML string to be inserted right before closing the
+.Aq BODY
+element.
 .It Ev CUSTOMIZE_TITLE
 An ASCII string to be used for the HTML
 .Aq TITLE

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

[Ingo Schwarze]

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