[gs-devel] Re: [gs-cvs] rev 7133 - trunk/gs/src

[SaGS]

----- Original Message ----- 
From: Stefan Kemper 
To: SaGS 
Cc: gs-devel ; Miles Jones ; Henry Stiles 
Sent: Tuesday, 31 October 2006 16:32
Subject: Re: [gs-devel] Re: [gs-cvs] rev 7133 - trunk/gs/src

> Dear SaGS,
>
> Please send me your patch, and the command line arguments that you 
> used to test it.
> ...

Dear Stefan,

As I understand the bug is already fixed in your working copy, I'll 
post the requested details and attach a diff here, not on Bugzilla.


(1) In GS_OPTIONS, quotes are treated as regular characters, 
    and not used for quoting spaces.

    gs\BIN>set GS_OPTIONS=-sDEVICE=nullpage                   <-/
                          -sTEST="HAVE -c TEST == SPACES"
    gs\BIN>gswin32c
    AFPL Ghostscript SVN PRE-RELEASE 8.55 (2006-05-20)
    Copyright (C) 2006 artofcode LLC, Benicia, CA.  All rights reserved.
    This software comes with NO WARRANTY: see the file PUBLIC for details.
    ("HAVE)
    Error: /undefined in SPACES"
    ...

Expected behaviour:
  - use nullpage as initial device;
    Note: not related to this bug, but otherwise GS tries to install
          an initial device and fails. Don't know why; in (2) below 
          it doesn't.
  - set /TEST to <(HAVE -c TEST == SPACES)>  (I'm using "<>" for 
    citing, to avoid confusion with quotes in cited text);
  - display the GS prompt (allows to examine /TEST).

Actual behaviour:
  - use nullpage as initial device;
  - set /TEST to <"HAVE> (note: with a leading quote);
  - find <-c> and execute <TEST>, <==>, then <SPACES"> (note: last one
    has a trailing quote). The <-c> inside the string is only to show 
    what /TEST has been set to, and is not connected to (2) below.


(2) Quoting ineffective for 1st switch following <-c>.

    gs\BIN>gswin32c -c 1 pop -sTEST="HAVE -c TEST == SPACES"
    AFPL Ghostscript SVN PRE-RELEASE 8.55 (2006-05-20)
    Copyright (C) 2006 artofcode LLC, Benicia, CA.  All rights reserved.
    This software comes with NO WARRANTY: see the file PUBLIC for details.
    (HAVE)
    Error: /undefined in SPACES
    ...

Expected behaviour:
  - execute <1>, then <pop>;
  - define /TEST as <(HAVE -c TEST == SPACES)>;
  - display the GS prompt (allows to examine /TEST).

Actual behaviour:
  - execute <1>, then <pop>;
  - define /TEST as <(HAVE)>;
  - find <-c> and execute <TEST>, <==>, then <SPACES>.

Differs from (1) above: text is parsed twice. 1st pass finds and removes 
the quotes; during the 2nd parsing quotes aren't there anymore, and 
spaces that were previously protected are treated as delimiters.


(3) Quoting ineffective for GhostPCL's (1.41) input filenames.
    Note: Does not affect Ghostscript.

Any input file with spaces in the name or path (as typed on the 
command line, not after converting to a full path), even properly 
enclosed in <"">, fails as if the <""> were not there.


My [short] explanation of the cause:

In the situations above, some text gets "pushed back" into the 
arguments list. But:

(a) Sometimes this text represents a single [already parsed] argument, 
    retrieved by a previous call to arg_next(). In this case, quotes 
    were already found and removed, spaces are part of the argument 
    itself and not a delimiter. When this text is encountered again 
    it should be taken as-is.

(b) Other times (GS_OPTIONS) this text is an [unparsed] list of 
    arguments; [only the] unquoted spaces are delimiters. When 
    arg_next() reaches the string it has to parse it normally.

(c) Currently arg_next() cannot know if it's case (a) or (b). It does 
    the parsing from the <spaces are delimiters> point of view, but 
    does not do the parsing from the <quotes are for quoting> point of 
    view. IMHO, the <&& f != NULL> (hunk <@@ -172,9 +189,9 @@> in the 
    attached diff) was an [incorrect and partial] attempt to 
    handle (a). Taking <"> as a regular character in GS_OPTIONS is a 
    direct consequence of this. I see absolutely no reason for 
    accepting spaces as delimiters without allowing <"> to quote them.


Sincerely yours,
Gh. Savulescu
-------------- next part --------------
Index: src/gsargs.h
===================================================================
--- src/gsargs.h	(revision 7138)
+++ src/gsargs.h	(working copy)
@@ -29,6 +29,7 @@
     bool is_file;
     union _u {
 	struct _su {
+	    bool parsed;	/* true for "pushed-back" argument, not to be parsed again */
 	    char *chars;	/* original string */
 	    gs_memory_t *memory;  /* if non-0, free chars when done with it */
 	    const char *str;	/* string being read */
@@ -57,13 +58,11 @@
  * This may also be used (once) to "unread" the last argument.
  * If mem != 0, it is used to free the string when we are done with it.
  * Return 0 on success, non-zero on failure
- * NB: pushing args has the side effect of changing the parsing algoritm to
- * space delimited instead of argument string delimited.
  */
-int arg_push_memory_string(arg_list * pal, char *str, gs_memory_t * mem);
+int arg_push_memory_string(arg_list * pal, char *str, bool parsed, gs_memory_t * mem);
 
-#define arg_push_string(pal, str)\
-  arg_push_memory_string(pal, str, (gs_memory_t *)0);
+#define arg_push_string(pal, str, parsed)\
+  arg_push_memory_string(pal, str, parsed, (gs_memory_t *)0);
 
 /* Clean up an arg list before exiting. */
 void arg_finit(arg_list * pal);



Index: src/imainarg.c
===================================================================
--- src/imainarg.c	(revision 7138)
+++ src/imainarg.c	(working copy)
@@ -194,7 +194,7 @@
 	    (char *)gs_alloc_bytes(minst->heap, len, "GS_OPTIONS");
 
 	    gp_getenv(GS_OPTIONS, opts, &len);	/* can't fail */
-	    if (arg_push_memory_string(&args, opts, minst->heap))
+	    if (arg_push_memory_string(&args, opts, false, minst->heap))
 		return e_Fatal;
 	}
     }
@@ -366,7 +366,7 @@
 		    char *p = arg_copy(arg, minst->heap);
 		    if (p == NULL)
 			return e_Fatal;
-		    arg_push_string(pal, p);
+		    arg_push_string(pal, p, true);
 		}
 		pal->expand_ats = ats;
 		break;



Index: src/gsargs.c
===================================================================
--- src/gsargs.c	(revision 7138)
+++ src/gsargs.c	(working copy)
@@ -37,7 +37,7 @@
 
 /* Push a string onto an arg list. */
 int
-arg_push_memory_string(arg_list * pal, char *str, gs_memory_t * mem)
+arg_push_memory_string(arg_list * pal, char *str, bool parsed, gs_memory_t * mem)
 {
     arg_source *pas;
 
@@ -47,6 +47,7 @@
     }
     pas = &pal->sources[pal->depth];
     pas->is_file = false;
+    pas->u.s.parsed = parsed;
     pas->u.s.chars = str;
     pas->u.s.memory = mem;
     pas->u.s.str = str;
@@ -92,6 +93,22 @@
     }
     if (pas->is_file)
 	f = pas->u.file, endc = EOF;
+    else if (pas->u.s.parsed)
+	/* this	string is a "pushed-back" argument		     */
+	/* (retrieved by a precedeing arg_next(), but not processed) */
+	if (strlen(pas->u.s.str) >= arg_str_max) {
+	    errprintf("Command too long: %s\n", pas->u.s.str);
+	    *code = gs_error_Fatal;
+	    return NULL;
+	} else {
+	    strcpy(pal->cstr, pas->u.s.str);
+	    result = pal->cstr;
+	    if (pas->u.s.memory)
+		gs_free_object(pas->u.s.memory,	pas->u.s.chars,	"arg_next");
+	    pal->depth--;
+	    pas--;
+	    goto at;
+	}
     else
 	astr = pas->u.s.str, f = NULL, endc = 0;
     result = cstr = pal->cstr;
@@ -172,9 +189,9 @@
 	    *code = gs_error_Fatal;
 	    return NULL;
 	}
-	/* If input is coming from an @-file, allow quotes */
-	/* to protect whitespace. */
-	if (c == '"' && f != NULL)
+	/* Allow quotes to protect whitespace. */
+	/* (special cases have already been handled and don't reach this point) */
+	if (c == '"')
 	    in_quote = !in_quote;
 	else
 	    cstr[i++] = c;