OpenPrinting / cups-browsed

Apache License 2.0
6 stars 4 forks source link

[Bug report & fix] the location of driverless printer is removed after reboot. #16

Closed cchoongh closed 1 year ago

cchoongh commented 3 years ago

OS: Debian buster CUPS Version: 2.2.10

Hi, please understand that I'm not a native speaker.

I think the code below should be deleted. https://github.com/OpenPrinting/cups-filters/blob/7999624bd2d2e3297c44f88d7070f6f44f6db31d/utils/cups-browsed.c#L8606-L8608

Because, 1) the printer-location option is loaded to p->options at https://github.com/OpenPrinting/cups-filters/blob/7999624bd2d2e3297c44f88d7070f6f44f6db31d/utils/cups-browsed.c#L8279-L8281 2) after that, p->options is copied to options at line 8622 ~ 8627 and encoded to IPP request at line 8628 ~ 8630. https://github.com/OpenPrinting/cups-filters/blob/7999624bd2d2e3297c44f88d7070f6f44f6db31d/utils/cups-browsed.c#L8622-L8630

But, If the printer-location option was added to IPP request in advance like line 8606 ~ 8608, the printer-location option is ignored at the cupsEncodeOptions2 fuction(line 8628 ~ 8630) because it is in IPP request already.

As a result, after reboot, the printer-location value in /var/cache/cups/cups-browsed-options-* is became empty because, from what I've seen so far, the p->location is always empty string.

(and I also think this should be deleted too, because of needless https://github.com/OpenPrinting/cups-filters/blob/7999624bd2d2e3297c44f88d7070f6f44f6db31d/utils/cups-browsed.c#L8618-L8620)

tillkamppeter commented 3 years ago

Thank you very much, Applied as commit ebc50cb99 on the master branch and as commit 522dcfaca002 on the 1.x branch.

efuss commented 1 year ago

This breaks distribution of printer-location and printer-info fields from a central CUPS server I introduced in 2017 in what became commits 175feefa5995e62e7407ed548e5f0e02b8dd4109 and 7d29177f576e4a430dcead54c2ed98436696c4fb.

tillkamppeter commented 1 year ago

Looks like that we should re-introduce these ones here, but with a chack whether description and location are actually non-empty, like:

 /* Location */
  if (p->location && p->location[0])
    ippAddString(request, IPP_TAG_PRINTER, IPP_TAG_TEXT,
                 "printer-location", NULL, p->location); 
  [...]
 /* Description */
  if (p->info && p->info[0])
    num_options = cupsAddOption("printer-info", p->info, 
                                num_options, &options);

Then, in case of a central CUPS server where description and/or location are populated they get used, and if there is no description and/or location supplied by the server, one can define them on the client side and they stay conserved. WDYT? Could you perhaps even test it?

tillkamppeter commented 1 year ago

Fixed via commit eae7971.

Thank you very much Edgar Fuß!

tillkamppeter commented 1 year ago

Also fixed in the 1.x branch, commit https://github.com/OpenPrinting/cups-filters/commit/d72184e7