lprng-devel Mailing List for LPRng

* walter harms <wh...@bf...> [120625 15:42]:
> we should die in glory and do
> 
> 	xasprintf(&pathname,"%s/temp%02dXXXXXX",dir,Tempfiles.count );
>[...]
> 
> since posix 2008 asprintf() is ok (and we could provide an alternative if needed)
> an other nice target (== more work) is dprintf() also POSIX now.

I've looked into this and extentening plp_snprintf to have a plp_asnprintf would
be (without adding some more glue code for some filters only using
plp_snprintf).

 src/Makefile.am            |    8 +--
 src/common/linelist.c      |  217 --------------------------------------------------------------
 src/common/memory.c        |  229 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/common/plp_snprintf.c  |  184 +++++++++++++++++++++++++++++++++++------------------
 src/include/linelist.h     |   20 +-----
 src/include/memory.h       |   32 ++++++++++
 src/include/plp_snprintf.h |    7 ++
 7 files changed, 395 insertions(+), 302 deletions(-)

(most are quite mechanical changes, though).

One could of course also look into replacing plp_*printf with the
generic printf functions (and some of the many fallback codes for
asprintf), which would involve:

 - making sure only portable printf features are used (you cannot give
   NULL for %s, ....)
 - make sure none of the security features built into plp_snprintf are
   needed or how they can be replaced with other safeguards.
 - how to handle the quoting of meta-characters instead
 - lose the ability to debug where memory is allocated and where we run
   out of it (or having to write replacement code).

All in all, I thought this change was much easier...

        Bernhard R. Link

Am 25.06.2012 14:06, schrieb Bernhard R. Link:
> I've pushed the following patch:
> 
> From: "Bernhard R. Link" <br...@de...>
> Date: Fri, 22 Jun 2012 17:29:25 +0200
> Subject: [PATCH] get rid of MAXPATHLEN usage
> 
> avoid usung file length limits supporting systems without such limits.
> ---
>  ChangeLog              |    1 +
>  src/auth/ssl_auth.c    |    2 +-
>  src/common/linelist.c  |    8 ++++++--
>  src/common/user_auth.c |   15 ++++++++++++---
>  src/include/portable.h |    6 ------
>  5 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index a2925e6..09c3d62 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,4 @@
> +2012-06-22 brl get rid of MAXPATHLEN usage
>  2011-10-26 brl drop --with-ssl, use autoconf macros to look for libssl
>  2011-10-26 brl set PPD in print filters' environment, if printcap has ppd= set
>  2011-10-26 brl fix typo breaking kerberos plugin built (except on x86_32)
> diff --git a/src/auth/ssl_auth.c b/src/auth/ssl_auth.c
> index 2297cf0..ce3b439 100644
> --- a/src/auth/ssl_auth.c
> +++ b/src/auth/ssl_auth.c
> @@ -192,7 +192,7 @@ static int SSL_Initialize_ctx(
>      SSL_METHOD *meth = 0;
>      SSL_CTX *ctx = 0;
>  	char header[SMALLBUFFER];
> -	char cabuf[MAXPATHLEN], certbuf[MAXPATHLEN], pwbuf[MAXPATHLEN]; 
> +	char certbuf[4096], pwbuf[4096]; 
>  	struct stat statb;
>  	int pid;
>  	char *file, *s;
> diff --git a/src/common/linelist.c b/src/common/linelist.c
> index 1cd87bc..a35f829 100644
> --- a/src/common/linelist.c
> +++ b/src/common/linelist.c
> @@ -2375,9 +2375,12 @@ int Make_temp_fd_in_dir( char **temppath, char *dir )
>  {
>  	int tempfd;
>  	struct stat statb;
> -	char pathname[MAXPATHLEN];
> +	int len;
> +	char *pathname;
>  
> -	plp_snprintf(pathname,sizeof(pathname), "%s/temp%02dXXXXXX",dir,Tempfiles.count );
> +	len = 1 + plp_snprintf(NULL, 0, "%s/temp%02dXXXXXX",dir,Tempfiles.count );
> +	pathname = malloc_or_die(len, __FILE__, __LINE__);
> +	plp_snprintf(pathname, len, "%s/temp%02dXXXXXX",dir,Tempfiles.count );

we should die in glory and do

	xasprintf(&pathname,"%s/temp%02dXXXXXX",dir,Tempfiles.count );

where xasprintf is like

	ret=asprintf(&buf,-...):
	if (ret<0)
		panic("no enought mememory");

	return buf;

since posix 2008 asprintf() is ok (and we could provide an alternative if needed)
an other nice target (== more work) is dprintf() also POSIX now.

re,
 wh





>  	tempfd = mkstemp( pathname );
>  	if( tempfd == -1 ){
>  		Errorcode = JFAIL;
> @@ -2397,6 +2400,7 @@ int Make_temp_fd_in_dir( char **temppath, char *dir )
>  		logerr_die(LOG_INFO, "Make_temp_fd_in_dir: stat '%s' failed ", pathname );
>  	}
>  	DEBUG1("Make_temp_fd_in_dir: fd %d, name '%s'", tempfd, pathname );
> +	free(pathname);
>  	return( tempfd );
>  }
>  
> diff --git a/src/common/user_auth.c b/src/common/user_auth.c
> index 3e07bd0..b659373 100644
> --- a/src/common/user_auth.c
> +++ b/src/common/user_auth.c
> @@ -195,11 +195,18 @@ static void LoadSecurityPlugin(const char *name) {
>  
>  		i = lstat(filename, &s);
>  		if( i == 0 && S_ISLNK(s.st_mode) ) {
> +			size_t reserved = 4096;
>  			ssize_t linklen;
> -			char linkname[MAXPATHLEN];
> +			char *linkname = malloc_or_die(reserved,
> +					__FILE__, __LINE__);
>  
> -			linklen = readlink(filename, linkname, sizeof(linkname));
> -			if( linklen > 0 || linklen <sizeof(linkname) ) {
> +			linklen = readlink(filename, linkname, reserved);
> +			while( linklen >= reserved ) {
> +				reserved = linklen + 1;
> +				linkname = realloc_or_die(linkname, reserved,
> +						__FILE__, __LINE__);
> +			}
> +			if( linklen > 0 && linklen < reserved ) {
>  				char *realname, *ee;
>  
>  				linkname[linklen] = '\0';
> @@ -214,10 +221,12 @@ static void LoadSecurityPlugin(const char *name) {
>  					*ee = '\0';
>  				if( loadplugin(filename, realname) ) {
>  					free(filename);
> +					free(linkname);
>  					return;
>  				}
>  
>  			}
> +			free(linkname);
>  		} else if( i == 0 && S_ISREG(s.st_mode) ) {
>  			if( loadplugin(filename, name) ) {
>  				free(filename);
> diff --git a/src/include/portable.h b/src/include/portable.h
> index 83e62a7..d68d311 100644
> --- a/src/include/portable.h
> +++ b/src/include/portable.h
> @@ -98,12 +98,7 @@ LPRng requires ANSI Standard C compiler
>  /*************************************************************************
>   * SCO OpenServer 5.0.5
>   *************************************************************************/
> -/* normal include files do not define MAXPATHLEN - rather PATHSIZE in 
> -   sys/param.h */
>  #ifdef sco
> -#ifndef MAXPATHLEN
> -#define MAXPATHLEN	PATHSIZE
> -#endif
>  /* SCO doesn't define the S_ISSOCK POSIX macro to use in testing the 
>     stat.st_mode structure member  - it appears as though a socket has
>     st_mode = 0020000 (same as character special) */
> @@ -118,7 +113,6 @@ LPRng requires ANSI Standard C compiler
>   *************************************************************************/
>  
>  #if defined(cray)
> -#define MAXPATHLEN	1023
>  #define HAVE_SIGLONGJMP	1
>  
>  /* configure incorrectly chooses STATVFS */

I've prepared a release candidate 1 of 3.8.C.

Please download and test...

The following would be the release notes
(not yet pushed):

From: "Bernhard R. Link" <br...@de...>
Date: Mon, 25 Jun 2012 11:49:49 +0200
Subject: [PATCH] release 3.8.C

---
 NEWS         |   10 ++++++++++
 configure.ac |    2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 65887a1..b25580d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,14 @@
 ---------------------------------------------------------------------
+Version LPRng-3.8.C
+---------------------------------------------------------------------
+
+- some code cleanups (don't use MAXPATHLEN, don't failt with gcc's
+  -Werror=format-security, ...)
+- improve some configure checks
+- set PPD variable in filters' environment if ppd= in printcap set
+- fix some issues with --enable-plugins
+
+---------------------------------------------------------------------
 Version LPRng-3.8.B
 ---------------------------------------------------------------------
 
diff --git a/configure.ac b/configure.ac
index 0e85c95..dfd64c6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,4 +1,4 @@
-AC_INIT(LPRng,3.8.C~snapshot,lpr...@li...)
+AC_INIT(LPRng,3.8.C,lpr...@li...)
 AC_CONFIG_SRCDIR(src/common/lpr.c)
 AM_CONFIG_HEADER(config.h)
 AM_INIT_AUTOMAKE([foreign])
-- 
1.7.10

I've pushed the following patch:

From: "Bernhard R. Link" <br...@de...>
Date: Sat, 23 Jun 2012 09:23:17 +0200
Subject: [PATCH] fix some issues in plugins autoconf check

add AC_LANG_SOURCE to AC_LINK_IFELSE check. This should only
add some defines not used but silences autoconf complaining about
it not being used.

also fix some misspelling of a variable which might cause systems
where plugins cannot be used to not be detected as such.
---
 ChangeLog    |    1 +
 acinclude.m4 |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 09c3d62..022b3da 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,4 @@
+2012-06-23 brl fix some issues in configure check for plugins
 2012-06-22 brl get rid of MAXPATHLEN usage
 2011-10-26 brl drop --with-ssl, use autoconf macros to look for libssl
 2011-10-26 brl set PPD in print filters' environment, if printcap has ppd= set
diff --git a/acinclude.m4 b/acinclude.m4
index 739c652..4797d74 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -115,14 +115,14 @@ mysaved_LDFLAGS="$LDFLAGS"
 mysaved_CFLAGS="$CFLAGS"
 LDFLAGS="$LDFLAGS $PLUGIN_LDFLAGS"
 CFLAGS="$CFLAGS $PLUGIN_CFLAGS"
-AC_LINK_IFELSE([[int test(void);int test(void) {return callback();}]],
+AC_LINK_IFELSE([AC_LANG_SOURCE([[int test(void);int test(void) {return callback();}]])],
 [cp conftest$ac_exeext libmyXYZtest.so || AC_MSG_ERROR([Internal error, perhaps autoconf changed soem internals])
  my_cv_sys_shared_with_callback_works=yes],
 [my_cv_sys_shared_with_callback_works=no])
 if test $my_cv_sys_shared_with_callback_works = yes ; then
 	LDFLAGS="$mysaved_LDFLAGS $PLUGINUSER_LDFLAGS -L. -lmyXYZtest"
 	CFLAGS="$mysaved_CFLAGS"
-	AC_TRY_LINK([int callback(void) { return 17; } return test();], [my_cv_sys_shared_with_callback_works=yes], [my_cv_sys_shared_works=no])
+	AC_TRY_LINK([int callback(void) { return 17; } return test();], [my_cv_sys_shared_with_callback_works=yes], [my_cv_sys_shared_with_callback_works=no])
 fi
 LDFLAGS="$mysaved_LDFLAGS"
 CFLAGS="$mysaved_CFLAGS"
-- 
1.7.10

I've pushed the following patch:

From: "Bernhard R. Link" <br...@de...>
Date: Fri, 22 Jun 2012 17:29:25 +0200
Subject: [PATCH] get rid of MAXPATHLEN usage

avoid usung file length limits supporting systems without such limits.
---
 ChangeLog              |    1 +
 src/auth/ssl_auth.c    |    2 +-
 src/common/linelist.c  |    8 ++++++--
 src/common/user_auth.c |   15 ++++++++++++---
 src/include/portable.h |    6 ------
 5 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a2925e6..09c3d62 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,4 @@
+2012-06-22 brl get rid of MAXPATHLEN usage
 2011-10-26 brl drop --with-ssl, use autoconf macros to look for libssl
 2011-10-26 brl set PPD in print filters' environment, if printcap has ppd= set
 2011-10-26 brl fix typo breaking kerberos plugin built (except on x86_32)
diff --git a/src/auth/ssl_auth.c b/src/auth/ssl_auth.c
index 2297cf0..ce3b439 100644
--- a/src/auth/ssl_auth.c
+++ b/src/auth/ssl_auth.c
@@ -192,7 +192,7 @@ static int SSL_Initialize_ctx(
     SSL_METHOD *meth = 0;
     SSL_CTX *ctx = 0;
 	char header[SMALLBUFFER];
-	char cabuf[MAXPATHLEN], certbuf[MAXPATHLEN], pwbuf[MAXPATHLEN]; 
+	char certbuf[4096], pwbuf[4096]; 
 	struct stat statb;
 	int pid;
 	char *file, *s;
diff --git a/src/common/linelist.c b/src/common/linelist.c
index 1cd87bc..a35f829 100644
--- a/src/common/linelist.c
+++ b/src/common/linelist.c
@@ -2375,9 +2375,12 @@ int Make_temp_fd_in_dir( char **temppath, char *dir )
 {
 	int tempfd;
 	struct stat statb;
-	char pathname[MAXPATHLEN];
+	int len;
+	char *pathname;
 
-	plp_snprintf(pathname,sizeof(pathname), "%s/temp%02dXXXXXX",dir,Tempfiles.count );
+	len = 1 + plp_snprintf(NULL, 0, "%s/temp%02dXXXXXX",dir,Tempfiles.count );
+	pathname = malloc_or_die(len, __FILE__, __LINE__);
+	plp_snprintf(pathname, len, "%s/temp%02dXXXXXX",dir,Tempfiles.count );
 	tempfd = mkstemp( pathname );
 	if( tempfd == -1 ){
 		Errorcode = JFAIL;
@@ -2397,6 +2400,7 @@ int Make_temp_fd_in_dir( char **temppath, char *dir )
 		logerr_die(LOG_INFO, "Make_temp_fd_in_dir: stat '%s' failed ", pathname );
 	}
 	DEBUG1("Make_temp_fd_in_dir: fd %d, name '%s'", tempfd, pathname );
+	free(pathname);
 	return( tempfd );
 }
 
diff --git a/src/common/user_auth.c b/src/common/user_auth.c
index 3e07bd0..b659373 100644
--- a/src/common/user_auth.c
+++ b/src/common/user_auth.c
@@ -195,11 +195,18 @@ static void LoadSecurityPlugin(const char *name) {
 
 		i = lstat(filename, &s);
 		if( i == 0 && S_ISLNK(s.st_mode) ) {
+			size_t reserved = 4096;
 			ssize_t linklen;
-			char linkname[MAXPATHLEN];
+			char *linkname = malloc_or_die(reserved,
+					__FILE__, __LINE__);
 
-			linklen = readlink(filename, linkname, sizeof(linkname));
-			if( linklen > 0 || linklen <sizeof(linkname) ) {
+			linklen = readlink(filename, linkname, reserved);
+			while( linklen >= reserved ) {
+				reserved = linklen + 1;
+				linkname = realloc_or_die(linkname, reserved,
+						__FILE__, __LINE__);
+			}
+			if( linklen > 0 && linklen < reserved ) {
 				char *realname, *ee;
 
 				linkname[linklen] = '\0';
@@ -214,10 +221,12 @@ static void LoadSecurityPlugin(const char *name) {
 					*ee = '\0';
 				if( loadplugin(filename, realname) ) {
 					free(filename);
+					free(linkname);
 					return;
 				}
 
 			}
+			free(linkname);
 		} else if( i == 0 && S_ISREG(s.st_mode) ) {
 			if( loadplugin(filename, name) ) {
 				free(filename);
diff --git a/src/include/portable.h b/src/include/portable.h
index 83e62a7..d68d311 100644
--- a/src/include/portable.h
+++ b/src/include/portable.h
@@ -98,12 +98,7 @@ LPRng requires ANSI Standard C compiler
 /*************************************************************************
  * SCO OpenServer 5.0.5
  *************************************************************************/
-/* normal include files do not define MAXPATHLEN - rather PATHSIZE in 
-   sys/param.h */
 #ifdef sco
-#ifndef MAXPATHLEN
-#define MAXPATHLEN	PATHSIZE
-#endif
 /* SCO doesn't define the S_ISSOCK POSIX macro to use in testing the 
    stat.st_mode structure member  - it appears as though a socket has
    st_mode = 0020000 (same as character special) */
@@ -118,7 +113,6 @@ LPRng requires ANSI Standard C compiler
  *************************************************************************/
 
 #if defined(cray)
-#define MAXPATHLEN	1023
 #define HAVE_SIGLONGJMP	1
 
 /* configure incorrectly chooses STATVFS */
-- 
1.7.10

* walter harms <wh...@bf...> [120427 18:07]:
> Is there a defined behavior regarding zero sized files ?
>
> background:
> was was chasing a strange bug in busybox lpr. It can send zero sized files,
> and expects a answer that never comes.
>
> The problem did not show up earlier because CUPS lpr and lprng LPR do not
> send zero size files. unfortunately i could not find a hint in rfc1179 that
> specifies that this is forbidden.

The RFC says about data files transmitted:

'The total number of bytes in the stream may be sent as the first operand, otherwise
the field should be cleared to 0.'

'If a data file length has been specified, the next "Operand 1"
octets over the same TCP connection are the intended contents of the
data file.  In this case, once all of the contents have been
delivered, an octet of zero bits is sent as an indication that the
file being sent is complete.  A second level of acknowledgement
processing must occur at this point.'

The way I read this (and the way lprng implements this), is:
A length field of "0" means the data file is till the end of the
input stream. If the busybox lpr sends a file advertising a length
of 0 and then 0 bytes followed by one zero byte, I think it cannot
except and answer (unless it closes the writing side of the socket),
as lprng must wait for more data to follow.

        Bernhard R. Link