Re: implement .so

[Ingo Schwarze <schwarze@usta.de>]

Hi Joerg,

Joerg Sonnenberger wrote on Sun, Oct 24, 2010 at 09:51:35PM +0200:

> As we have seen in Rostock, full .so implementation has some funny side
> effects, so I consider it a broken feature.

Yes, no doubt.  It is broken by design in multiple ways.

> I won't oppose a clean patch, we have already seen that it is
> straight forward to implement.

See below, feedback is welcome.
After the first round of review and polishing here
i'm planning to commit it to OpenBSD and have it looked
over by a few people round here.

> My point is that it essentially falls apart if you want to use
> compressed man pages.

Sure, i understand that.

> So it really, really shouldn't be used.

I fully agree.
This is to deal with existing stuff
and should not be used for new projects.

> Sorry, yes. Only allow relative path names without /../ components.

Actually, it is even worse.
All ports in the OpenBSD ports tree using the feature give the path
relative to the *parent* directory of the directory containing the
manual page having the .so directive.

So these are neither absolute paths (relative to the root of the
file system) nor relative paths (relative to the current working
directory).

What i'm doing for now is:

 * regarding validation of the .so directive (roff.c, roff_so()):
    - path must start with "man"
    - path must contain exactly one '/'
    - the characters before and after the slash must not be '.'
    - the slash must not be the last character
 * regarding validation of the path to the page containing
   the .so directive (main.c, pfile()):
    - path must contain at least one '/'
    - component before the slash must start with "man"

In this case, i replace the last two components of the path to
the page containing the .so directive by the contents of the
directive, or else error out.

Note that the patch builds upon the preceding refactoring patch
i sent yesterday, both building on the OpenBSD tree.

Yours,
  Ingo


diff -Napur mandoc.pdesc/main.c mandoc/main.c
--- mandoc.pdesc/main.c	Sun Oct 24 13:47:06 2010
+++ mandoc/main.c	Mon Oct 25 15:02:52 2010
@@ -15,6 +15,7 @@
  * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
+#include <sys/param.h>
 #include <sys/types.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
@@ -173,6 +174,8 @@ static	const char * const	mandocerrs[MANDOCERR_MAX] = 
 	"argument count wrong, violates syntax",
 	"child violates parent syntax",
 	"argument count wrong, violates syntax",
+	"invalid path in include directive",
+	"include used by a file at an invalid path",
 	"no document body",
 	"no document prologue",
 	"static buffer exhausted",
@@ -181,6 +184,7 @@ static	const char * const	mandocerrs[MANDOCERR_MAX] = 
 static	void		  pdesc(struct curparse *);
 static	void		  fdesc(struct curparse *);
 static	void		  ffile(const char *, struct curparse *);
+static	int		  pfile(const char *, struct curparse *, int);
 static	int		  moptions(enum intt *, char *);
 static	int		  mmsg(enum mandocerr, void *, 
 				int, int, const char *);
@@ -306,7 +310,58 @@ ffile(const char *file, struct curparse *curp)
 		perror(curp->file);
 }
 
+static int
+pfile(const char *file, struct curparse *curp, int ln)
+{
+	char		 path[MAXPATHLEN];
+	const char	*savefile;
+	char		*p;
+	int		 savefd;
 
+	if (strlcpy(path, curp->file, sizeof(path)) >= sizeof(path)) {
+		mmsg(MANDOCERR_MEM, curp, ln, 1, NULL);
+		return(0);
+	}
+
+	if (NULL == (p = strrchr(path, '/'))) {
+		mmsg(MANDOCERR_SOUSER, curp, ln, 1, NULL);
+		return(0);
+	}
+
+	while (path < p && '/' != p[-1])
+		p--;
+
+	if (strncmp(p, "man", 3)) {
+		mmsg(MANDOCERR_SOUSER, curp, ln, 1, NULL);
+		return(0);
+	}
+	*p = '\0';
+
+	if (strlcat(path, file, sizeof(path)) >= sizeof(path)) {
+		mmsg(MANDOCERR_MEM, curp, ln, 1, NULL);
+		return(0);
+	}
+
+	savefile = curp->file;
+	savefd = curp->fd;
+
+	curp->file = path;
+	if (-1 == (curp->fd = open(curp->file, O_RDONLY, 0))) {
+		perror(curp->file);
+		exit_status = MANDOCLEVEL_SYSERR;
+	} else {
+		pdesc(curp);
+		if (-1 == close(curp->fd))
+			perror(curp->file);
+	}
+
+	curp->file = savefile;
+	curp->fd = savefd;
+
+	return(MANDOCLEVEL_FATAL > exit_status ? 1 : 0);
+}
+
+
 static void
 resize_buf(struct buf *buf, size_t initial)
 {
@@ -629,6 +684,11 @@ pdesc(struct curparse *curp)
 		} else if (ROFF_ERR == re) {
 			assert(MANDOCLEVEL_FATAL <= exit_status);
 			break;
+		} else if (ROFF_SO == re) {
+			if (pfile(ln.buf + of, curp, lnn_start))
+				continue;
+			else
+				break;
 		}
 
 		/*
diff -Napur mandoc.pdesc/mandoc.h mandoc/mandoc.h
--- mandoc.pdesc/mandoc.h	Sun Oct 24 13:47:06 2010
+++ mandoc/mandoc.h	Mon Oct 25 14:57:22 2010
@@ -114,6 +114,8 @@ enum	mandocerr {
 	MANDOCERR_SYNTARGVCOUNT, /* argument count wrong, violates syntax */
 	MANDOCERR_SYNTCHILD, /* child violates parent syntax */
 	MANDOCERR_SYNTARGCOUNT, /* argument count wrong, violates syntax */
+	MANDOCERR_SOPATH, /* invalid path in include directive */
+	MANDOCERR_SOUSER, /* include used by a file at an invalid path */
 	MANDOCERR_NODOCBODY, /* no document body */
 	MANDOCERR_NODOCPROLOG, /* no document prologue */
 	MANDOCERR_MEM, /* static buffer exhausted */
diff -Napur mandoc.pdesc/roff.c mandoc/roff.c
--- mandoc.pdesc/roff.c	Wed Oct 20 09:22:11 2010
+++ mandoc/roff.c	Mon Oct 25 14:38:24 2010
@@ -48,11 +48,12 @@ enum	rofft {
 	ROFF_ie,
 	ROFF_if,
 	ROFF_ig,
+	ROFF_nr,
 	ROFF_rm,
+	ROFF_so,
 	ROFF_tr,
 	ROFF_cblock,
 	ROFF_ccond, /* FIXME: remove this. */
-	ROFF_nr,
 	ROFF_MAX
 };
 
@@ -128,6 +129,7 @@ static	int		 roff_res(struct roff *, 
 				char **, size_t *, int);
 static	void		 roff_setstr(struct roff *,
 				const char *, const char *);
+static	enum rofferr	 roff_so(ROFF_ARGS);
 static	char		*roff_strdup(const char *);
 
 /* See roff_hash_find() */
@@ -150,11 +152,12 @@ static	struct roffmac	 roffs[ROFF_MAX] = {
 	{ "ie", roff_cond, roff_cond_text, roff_cond_sub, ROFFMAC_STRUCT, NULL },
 	{ "if", roff_cond, roff_cond_text, roff_cond_sub, ROFFMAC_STRUCT, NULL },
 	{ "ig", roff_block, roff_block_text, roff_block_sub, 0, NULL },
+	{ "nr", roff_nr, NULL, NULL, 0, NULL },
 	{ "rm", roff_line, NULL, NULL, 0, NULL },
+	{ "so", roff_so, NULL, NULL, 0, NULL },
 	{ "tr", roff_line, NULL, NULL, 0, NULL },
 	{ ".", roff_cblock, NULL, NULL, 0, NULL },
 	{ "\\}", roff_ccond, NULL, NULL, 0, NULL },
-	{ "nr", roff_nr, NULL, NULL, 0, NULL },
 };
 
 static	void		 roff_free1(struct roff *);
@@ -1005,6 +1008,26 @@ roff_nr(ROFF_ARGS)
 	}
 
 	return(ROFF_IGN);
+}
+
+
+/* ARGSUSED */
+static enum rofferr
+roff_so(ROFF_ARGS)
+{
+	char *name, *p;
+
+	name = *bufp + pos;
+	if (strncmp(name, "man", 3) ||
+	    NULL == (p = strchr(name, '/')) ||
+	    '.' == p[-1] || '.' == p[1] || '\0' == p[1] ||
+	    strchr(p+1, '/')) {
+		(*r->msg)(MANDOCERR_SOPATH, r->data, ln, pos, NULL);
+		return(ROFF_ERR);
+	}
+
+	*offs = pos;
+	return(ROFF_SO);
 }
 
 
diff -Napur mandoc.pdesc/roff.h mandoc/roff.h
--- mandoc.pdesc/roff.h	Wed Oct 20 09:22:11 2010
+++ mandoc/roff.h	Mon Oct 25 03:48:01 2010
@@ -20,6 +20,7 @@
 enum	rofferr {
 	ROFF_CONT, /* continue processing line */
 	ROFF_RERUN, /* re-run roff interpreter with offset */
+	ROFF_SO, /* include another file */
 	ROFF_IGN, /* ignore current line */
 	ROFF_ERR /* badness: puke and stop */
 };
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv