After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 169909 - The printer dialog doesn't escape the printer name
The printer dialog doesn't escape the printer name
Status: RESOLVED WONTFIX
Product: GIMP
Classification: Other
Component: Plugins
2.2.x
Other Linux
: Normal normal
: 2.2
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2005-03-11 06:49 UTC by Nigel
Modified: 2008-01-15 12:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
this little change should fix the problem (642 bytes, patch)
2005-03-15 10:50 UTC, Sven Neumann
committed Details | Review

Description Nigel 2005-03-11 06:49:31 UTC
Distribution/Version: Debian Unstable

Create a printer with a charactor such as ( or ) in it. (ie "Mita-FS-1900-(KPDL-2)")
Open and try to print the picture to this printer

If you goto the printer setup in the printer dialog and place \'s in front of
the appropriate charactors then GIMP can print to it.
Comment 1 Sven Neumann 2005-03-11 11:23:34 UTC
Well, the dialog shouldn't escape the printer names, but either we need to
escape them before sending them to libgimpprint or gimpprint needs to be fixed
to escape the arguments passed to lpr. Without having looked at the code yet,
I'd say this is the job of gimpprint.

It would be important to know what version of gimpprint you are using.
Comment 2 Sven Neumann 2005-03-15 10:50:07 UTC
Created attachment 38741 [details] [review]
this little change should fix the problem

The attached patch should fix this problem but hasn't been tested yet. Your
testing efforts would be very much appreciated.
Comment 3 Sven Neumann 2005-03-22 12:22:41 UTC
Nigel, did you get a chance to test the patch? Does it fix the problem for you?
Comment 4 weskaggs 2005-03-22 16:10:41 UTC
Setting status to NEEDINFO
Comment 5 Sven Neumann 2005-04-05 11:10:34 UTC
NEEDINFO is not a good status for a bug report that has a patch attached which
is waiting to be applied.
Comment 6 Sven Neumann 2005-04-05 11:27:31 UTC
Setting milestone to 2.2 since this is a change that should go into the stable
branch as soon as it has been tested.
Comment 7 Sven Neumann 2005-04-09 19:44:57 UTC
With the kind help of Robert Krawitz, we figured out that this is the right
thing to do here. Fixed in both branches.

2005-04-09  Sven Neumann  <sven@gimp.org>

	* plug-ins/print/print.c: quote the shell command passed to execl().
	Fixes bug #169909.
Comment 8 Robert Krawitz 2005-04-09 20:21:38 UTC
Actually, I tested it and it turned out that that change is NOT correct (at
least in 5.0, but 4.2 and 5.0 don't differ in that regard).  If it works in 4.2,
great, but otherwise here are my thoughts on this.  My email to Sven:

I put your fix in my 5.0 tree; it isn't working.  BTW, there's no
sense in trying to free something after an execl(); if execl() fails,
you just want to exit.

I think that the problem is that we're using execl, and we're passing
the command as a single word.

The unquoted command is lp -s -d esp2200  -oraw 
The quoted command is   'lp -s -d esp2200  -oraw '

The shell basically tries to parse the quoted command as a word.
Since it already has single quotes around it, the shell doesn't try to
interpret what's inside; it treats it as the name of a command, which
of course doesn't exist.  We don't want to pass the entire command as
an uninterpreted string; we want the shell to parse it, but to not
interpret any funny stuff inside.

This fix works in 5.0:

*** plist.c.~1.2.~	2005-01-27 22:02:44.000000000 -0500
--- plist.c	2005-04-09 15:31:28.834932597 -0400
***************
*** 195,201 ****
    stp_asprintf(&print_cmd, "%s %s %s %s %s%s%s",
  	       global_printing_system->print_command,
  	       queue_name[0] ? global_printing_system->queue_select : "",
! 	       queue_name[0] ? queue_name : "",
  	       count_string ? count_string : "",
  	       raw ? global_printing_system->raw_flag : "",
  	       extra_options ? " " : "",
--- 195,201 ----
    stp_asprintf(&print_cmd, "%s %s %s %s %s%s%s",
  	       global_printing_system->print_command,
  	       queue_name[0] ? global_printing_system->queue_select : "",
! 	       queue_name[0] ? g_shell_quote(queue_name) : "",
  	       count_string ? count_string : "",
  	       raw ? global_printing_system->raw_flag : "",
  	       extra_options ? " " : "",

It's going to be harder to fix it in 4.2, because stp_get_output_to()
has the name of the queue already embedded in it.  Probably the right
thing to do is to quote the printer queue name at the point where the
plugin extracts it from the lpc/lpstat command.  I haven't tested it;
when you test it, make sure that it doesn't keep adding more printers
every time someone runs the plugin.

Index: print.c
===================================================================
RCS file: /cvsroot/gimp-print/print/src/gimp/print.c,v
retrieving revision 1.22.4.4
diff -c -r1.22.4.4 print.c
*** print.c	24 Jan 2003 02:52:18 -0000	1.22.4.4
--- print.c	9 Apr 2005 19:38:35 -0000
***************
*** 1239,1244 ****
--- 1239,1245 ----
  	          line[0] != ' ' && line[0] != '\t')
                {
  		int printer_exists = 0;
+ 		char *quoted;
  		*ptr = '\0';
                  /* check for duplicate printers--yes, they can happen,
                   * and it makes gimp-print forget everything about the
***************
*** 1260,1266 ****
                  fprintf(stderr, "Adding new printer from lpc: <%s>\n",
                    line);
  #endif
! 		result = g_strdup_printf("lpr -P%s -l", line);
  		stp_set_output_to(plist[plist_count].v, result);
  		free(result);
  		stp_set_driver(plist[plist_count].v, "ps2");
--- 1261,1269 ----
                  fprintf(stderr, "Adding new printer from lpc: <%s>\n",
                    line);
  #endif
! 		quoted = g_shell_quote(line);
! 		result = g_strdup_printf("lpr -P%s -l", quoted);
! 		g_free(quoted);
  		stp_set_output_to(plist[plist_count].v, result);
  		free(result);
  		stp_set_driver(plist[plist_count].v, "ps2");
***************
*** 1274,1279 ****
--- 1277,1283 ----
  		  (sscanf(line, "Printer: %127s", name) == 1))
  	      {
  		int printer_exists = 0;
+ 		char *quoted;
                  /* check for duplicate printers--yes, they can happen,
                   * and it makes gimp-print forget everything about the
                   * printer */
***************
*** 1293,1299 ****
                  fprintf(stderr, "Adding new printer from lpc: <%s>\n",
                    name);
  #endif
! 		result = g_strdup_printf("lp -s -d%s -oraw", name);
  		stp_set_output_to(plist[plist_count].v, result);
  		free(result);
  		stp_set_driver(plist[plist_count].v, "ps2");
--- 1297,1305 ----
                  fprintf(stderr, "Adding new printer from lpc: <%s>\n",
                    name);
  #endif
! 		quoted = g_shell_quote(name);
! 		result = g_strdup_printf("lp -s -d%s -oraw", quoted);
! 		g_free(quoted);
  		stp_set_output_to(plist[plist_count].v, result);
  		free(result);
  		stp_set_driver(plist[plist_count].v, "ps2");
Comment 9 Sven Neumann 2005-04-09 23:49:30 UTC
Oh well. Here's what I am planning to do then: I will revert the change and
tomorrow release 2.2.6 without this bug being fixed. Someone will have to test
this patch then; otherwise the bug will remain unfixed in the 2.2 tree.
Comment 10 Sven Neumann 2005-04-09 23:55:30 UTC
2005-04-10  Sven Neumann  <sven@gimp.org>

	* plug-ins/print/print.c: reverted the previous change, it was wrong
	(see bug #169909).

Comment 11 Robert Krawitz 2005-04-10 04:01:48 UTC
I recommend not trying to fix this.  While it's a perfectly legal situation,
it's probably not all that common and there's an easy workaround -- quote the
printer name in the command.

I've already checked the above fix into Gutenprint 5.0-beta4, where the fix is
much more straightforward.  If someone wants to try my suggested fix, go ahead,
but I don't really have the infrastructure to test it properly and in any event
we're not planning to do another Gimp-Print 4.2 release.
Comment 12 Sven Neumann 2005-04-10 10:55:35 UTC
After all this hassle and since we didn't get any feedback from the bug
reporter, I am happily resolving this as WONTFIX.