OpenPrinting / cups

OpenPrinting CUPS Sources
https://openprinting.github.io/cups
Apache License 2.0
1.1k stars 195 forks source link

CUPS 2.4.11 refactor make-and-model code regression #1096

Closed dkosovic closed 1 week ago

dkosovic commented 1 week ago

The strip trailing whitespace code introduced with commit https://github.com/OpenPrinting/cups/commit/04bb2af4521b56c1699a2c2431c56c05a7102e69 seems broken.

If we start with a make-and-model string of "Hewlett Packard LaserJet 2024 Series", we end up with make="Hewlett" and model="Printer".

Below is standalone C test code that demonstrates how the strip trailing whitespace code introduced with that commit is broken, it's basically the same as the CUPS 2.4.11 code, but has some printf statements to see what's happening:

#include <stdio.h>
#include <string.h>

int main(int argc, char **argv) {
  char make[256],    /* Make and model */
      *mptr;         /* Pointer into make and model */

  strncpy(make, "Hewlett Packard LaserJet 2024 Series", sizeof(make));

  /*
   * Sanitize the model name to only contain PPD-safe characters.
   */

  for (mptr = make; *mptr; mptr++) {
    if (*mptr < ' ' || *mptr >= 127 || *mptr == '\"') {
      /*
       * Truncate the make and model on the first bad character...
       */

      *mptr = '\0';
      break;
    }
  }

  printf("strlen = %li, make = \"%s\"\n", mptr - make, make);
  while (mptr > make) {
    /*
     * Strip trailing whitespace...
     */

    mptr--;
    if (*mptr == ' ') {
      *mptr = '\0';
      printf("strlen = %li, make = \"%s\"\n", mptr - make, make);
    }
  }

  return 0;
}

The output from the above showing make progressively getting truncated on spaces:

strlen = 36, make = "Hewlett Packard LaserJet 2024 Series"
strlen = 29, make = "Hewlett Packard LaserJet 2024"
strlen = 24, make = "Hewlett Packard LaserJet"
strlen = 15, make = "Hewlett Packard"
strlen = 7, make = "Hewlett"

Although I didn't add test code for model, it is pretty obvious in the original code why model becomes "Printer" as there is no longer a separate model part left after the above:

https://github.com/OpenPrinting/cups/blob/7d535f73e407a72dfb5c5e83549a5ba27219dfcd/cups/ppd-cache.c#L3339-L3346

dkosovic commented 1 week ago

I replaced the existing strip trailing whitespaces while loop with the following and it seems to work correctly:

    mptr--;
    while (mptr > make && *mptr == ' ') {
        /*
         * Strip trailing whitespace
         */
        *mptr = '\0';
        mptr--;
    }
michaelrsweet commented 1 week ago

Yeah, we're missing an "else break" in the while loop, will re-refactor to fix this.

michaelrsweet commented 1 week ago

[master c1f619435] Fix make-and-model whitespace trimming (Issue #1096)

[2.4.x 5cc470c8d] Fix make-and-model whitespace trimming (Issue #1096)