OpenPrinting / cups

OpenPrinting CUPS Sources
https://openprinting.github.io/cups
Apache License 2.0
958 stars 174 forks source link

LPD backend mis-handles num_copies and manual_copies #917

Closed psz2036 closed 3 months ago

psz2036 commented 3 months ago

LPD backend mis-handles num_copies and manual_copies. Issue observed at version 2.4.2 in Debian bookworm, present still in current "master" version. Patch tested against Debian, but for current version, below (in plain-text and as attached file).

Cheers, Paul

Paul Szabo psz@maths.usyd.edu.au www.maths.usyd.edu.au/u/psz School of Mathematics and Statistics University of Sydney Australia

--- backend/lpd.c.ORIG  2024-03-31 08:19:57.994489623 +1100
+++ backend/lpd.c   2024-03-31 08:22:37.925544711 +1100
@@ -204,7 +204,14 @@
   format        = 'l';
   order         = ORDER_CONTROL_DATA;
   reserve       = RESERVE_ANY;
-  manual_copies = 1;
+  /* PSz 29 Feb 2024
+   * Set default manual_copies "off".
+   * With manual_copies "on", we simply run the copies together.
+   * Then a job of odd number of pages sent to a duplex printer,
+   * the first page of second copy gets printed on the back of the
+   * last page of the first copy.
+   */
+  manual_copies = 0;
   timeout       = 300;
   contimeout    = 7 * 24 * 60 * 60;

@@ -295,7 +302,7 @@
       else if (!_cups_strcasecmp(name, "mode") && value[0])
       {
        /*
-        * Set control/data order...
+        * Set mode...
    */

         if (!_cups_strcasecmp(value, "standard"))
@@ -341,6 +348,13 @@
        /*
         * Set manual copies...
    */
+   /* PSz 28 Feb 24
+    * Should not this be
+    *   cupsGetOption("manual_copies", num_jobopts, jobopts)
+    * or maybe some
+    *   ppd->manual_copies
+    * instead?
+    */

         manual_copies = !value[0] || !_cups_strcasecmp(value, "on") ||
            !_cups_strcasecmp(value, "yes") ||
@@ -387,7 +401,10 @@
     }
   }

-  if (mode == MODE_STREAM)
+  /* PSz 1 Mar 2024
+   * This override needed only if data from STDIN and in STREAM mode
+   */
+  if (argc == 6 && mode == MODE_STREAM)
     order = ORDER_CONTROL_DATA;

  /*
@@ -487,7 +504,13 @@
   * Queue the job...
   */

-  if (argc > 6)
+  /* PSz 27 Feb 2024
+   * Do not (needlessly) ignore number of copies requested.
+   * Surely can do when have file, whether named or our temporary.
+   * Can also do when data from STDIN and STREAM mode, though
+   * not in the manual_copies way.
+   */
+  if (argc > 6 || mode == MODE_STANDARD)
   {
     if (manual_copies)
     {
@@ -499,22 +522,16 @@
       manual_copies = 1;
       copies        = atoi(argv[4]);
     }
+  }

-    status = lpd_queue(hostname, addrlist, resource + 1, fd, snmp_fd, mode,
-                       username, title, copies, banner, format, order, reserve,
-              manual_copies, timeout, contimeout,
-              cupsGetOption("job-originating-host-name", num_jobopts,
-                            jobopts));
+  status = lpd_queue(hostname, addrlist, resource + 1, fd, snmp_fd, mode,
+                     username, title, copies, banner, format, order, reserve,
+            manual_copies, timeout, contimeout,
+            cupsGetOption("job-originating-host-name", num_jobopts,
+                          jobopts));

-    if (!status)
-      fprintf(stderr, "PAGE: 1 %d\n", atoi(argv[4]));
-  }
-  else
-    status = lpd_queue(hostname, addrlist, resource + 1, fd, snmp_fd, mode,
-                       username, title, 1, banner, format, order, reserve, 1,
-              timeout, contimeout,
-              cupsGetOption("job-originating-host-name", num_jobopts,
-                            jobopts));
+  if (!status)
+    fprintf(stderr, "PAGE: 1 %d\n", atoi(argv[4]));

  /*
   * Remove the temporary file if necessary...
@@ -948,6 +965,10 @@
     * Next, open the print file and figure out its size...
     */

+    /* PSz 1 Mar 2024
+     * Are we sure to get a non-zero print_fd when have file:
+     * do we "really know" that we were invoked with STDIN open?
+     */
     if (print_fd)
     {
      /*
@@ -1011,13 +1032,18 @@
       cptr   += strlen(cptr);
     }

-    while (copies > 0)
+    /* PSz 28 Feb 2024
+     * Check size remaining, do not blow with too many copies
+     */
+    while (copies > 0 && ((sizeof(control) - (size_t)(cptr - control)) > 256))
     {
       snprintf(cptr, sizeof(control) - (size_t)(cptr - control), "%cdfA%03d%.15s\n",
                format, (int)getpid() % 1000, localhost);
       cptr   += strlen(cptr);
       copies --;
     }
+    if (copies > 0)
+      fprintf(stderr, "DEBUG: Limited by control size, %d copies skipped\n", copies);

     snprintf(cptr, sizeof(control) - (size_t)(cptr - control),
              "UdfA%03d%.15s\n"

psz-patch.txt

michaelrsweet commented 3 months ago

This isn't correct.

When data is piped into a backend, the CUPS filter/backend interface REQUIRES that the upstream filters have produced any copies, doing whatever processing is necessary to ensure correct printing.

psz2036 commented 3 months ago

Requires filters to do copies, despite manual_copies being off? And then, why would backend even look at copies? Your code, your bug.