michaelrsweet / pappl

PAPPL - Printer Application Framework
https://www.msweet.org/pappl
Apache License 2.0
309 stars 49 forks source link

If validate_defaults() returns "false" nasty things happen #146

Closed tillkamppeter closed 3 years ago

tillkamppeter commented 3 years ago

pappl/printer-driver.c has the internal function validate_defaults() with which the default settings are checked for correctness (are they one of the available choices?) and if one incorrect default value is found, the function stops returning false, if it passes through all options without issue it returns true.

It is called by the papplPrinterSetDriverData() and papplPrinterSetDriverDefaults() functions.

In the PostScript Printer Application I ran into this function returning false via the sides option on a printer where the duplex unit is an installable accessory. With the duplex unit set as installed in the "Device Settings" I have set "two-sided-long-edge" as default for this option. Then I have set the duplex unit as not installed, making "two-sided-long-edge" not an available choice any more. Due to a bug in the PostScript Printer Application (is fixed now) the default did not get automatically corrected to "one-sided". Then I clicked "Media" in the web interface and the Printer Application crashed. In the log file (debug log level) I found out that the "sides" setting was unsupported, meaning that validate_defaults() returned false, probably in the papplPrinterSetDriverData() function which I call after changes in the installable options. This probably made the "media-ready" list not being filled in and this way I got the crash.

To get a simple reproducer I made the following change in testpappl:

diff --git a/testsuite/pwg-driver.c b/testsuite/pwg-driver.c
index 348636a..a9fd32d 100644
--- a/testsuite/pwg-driver.c
+++ b/testsuite/pwg-driver.c
@@ -305,7 +305,8 @@ pwg_callback(
     strlcpy(driver_data->media_ready[0].size_name, "na_letter_8.5x11in", sizeof(driver_data->media_ready[0].size_name));
     strlcpy(driver_data->media_ready[1].size_name, "iso_a4_210x297mm", sizeof(driver_data->media_ready[1].size_name));

-    driver_data->sides_supported = PAPPL_SIDES_ONE_SIDED | PAPPL_SIDES_TWO_SIDED_LONG_EDGE | PAPPL_SIDES_TWO_SIDED_SHORT_EDGE;
+    //driver_data->sides_supported = PAPPL_SIDES_ONE_SIDED | PAPPL_SIDES_TWO_SIDED_LONG_EDGE | PAPPL_SIDES_TWO_SIDED_SHORT_EDGE;
+    driver_data->sides_supported = PAPPL_SIDES_ONE_SIDED;
     driver_data->sides_default   = PAPPL_SIDES_TWO_SIDED_LONG_EDGE;
   }
   else

This causes exactly the same problem: "sides" has only "one-sided" available but the default is "two-sided-long-edge". I compiled and run it, then created a new printer with one of the "Office Printer" drivers and clicked on "Media" in the web interface, receiving a page with zero page sources, only a "Set" button, and "Printing Defaults" has an empty drop-down for the media, probably somehow with luck I did not get a crash here.

If validate_defaults() (or any other of the validate_...() functions) returns false, something more useful should happen, for example if setting defaults in the web interface causes it, stay on the "Printing Defaults" page and ggive an error message to the user, on printer creation, reject the driver, ....

michaelrsweet commented 3 years ago

[master 85ccdbd] Show an error if printing defaults are no good (Issue #146)

[v1.0.x 96198f9] Show an error if printing defaults are no good (Issue #146)