papplSystemLoadState() only loads first 4096 bytes of state file

[tillkamppeter]

With the PostScript Printer Application I can create and configure two printers without any problems, but when I create a third printer at least some of the settings get lost when stopping and restarting the Printer Application.

I have checked and found out that this gets caused by papplSystemLoadState() only loading the first 4096 bytes of the state file and with theree printers set up the state file gets longer, making some settings not getting read by papplSystemLoadState().

I looked into the code and did not find anything which could cause it. It can even be possible that the bug is in CUPS, in the function to read lines in configuration files.

I am using latest GIT snapshots of PAPPL, OpenPrinting CUPS, cups-filters (master branch, 2.x) and the PostScript Printer Application.

[michaelrsweet]

@tillkamppeter Please attach a state file that shows this problem.

[tillkamppeter]

This is independent of actual state file content, if the file is longer than 4096 bytes, only the first 4096 bytes get read.

[michaelrsweet]

@tillkamppeter I still need to see a file this is failing on. I CANNOT REPRODUCE THE PROBLEM.

[tillkamppeter]

Here is an example: ps-printer-app.state-more-than-4096.txt When using this as the state file for the PostScript Printer Application, you see that all default settings, both for installabel options ("Device Settings") and general defaults ("Printing Defaults") in the web interface correctly correspond to the settings saved in the file for the printers test and test2. On test3 I have set the RAM to 512 MB, the Input Tray Options to 2 Tray/ADU, and both Inverter Unit and Scanner Unit to "Installed". In the web interface under "Device Settings" you see these changes ignored: 384 MB RAM, no Input Tray Options, no inverter unit, no scanner unit. This is because reading of the file stops at 4096 bytes and so the line

installable-options-default InstalledMemory=512MB Option2=2TrayDrawer Option5=True Option1=NotInstalled Option9=False Option4=False OptionS=True

near the end of the state file got ignored. With the printer test2 you can see that this is not a problem of the method of handling the installable options but actually incomplete reading of the state file.

[tillkamppeter]

For testing the above state file you also need the PPD. Here we go: shc260fj.ppd.txt You can either manually put it into a directory where the Printer Application takes its PPDs from or use the "Add PPD files" button, then stop the Printer Application, replace its state file with the one I attached above, and start it again.

[tillkamppeter]

And here is the PPD file of the first printer: hp8730ps-query-en.ppd.txt

[michaelrsweet]

@tillkamppeter So there are multiple problems with how you have encoded these installable options.

First, text values in IPP cannot exceed 1023 bytes in length. CUPS' implementation of IPP allows values up to 32767 bytes (which is the limit of the encoding) but if you encode an xxx-default attribute with a value that long it will not be readable by all IPP clients. So I will be adding code to validate the length/type/content of the attributes you add and block any that cannot be validated.

Second, the issue is not that the state file cannot exceed 4096 bytes, it is that a single line cannot exceed 4096 bytes. I should implement a new read-line function that errors out on over-long lines.

[tillkamppeter]

At least the installable-options-default ... lines of my current state file do not exceed a length of 1023 bytes for the txt value or 4096 for the full line. The value is curremtly in the order of 100 bytes for the Sharp printer. I do not actually know whether there is a printer with that many installable accessories with appropriate options in the PPD file (~70).

The longest lines currently are the <Printer ...> lines with little more than 200 bytes.

[michaelrsweet]

This is odd, because the CUPS unit tests for files include reading a text file that is almost 32k in length (over the 4096 byte limit) using the cupsFileGets API I am using to read lines from the state file.

[michaelrsweet]

@tillkamppeter Unable to reproduce with testpappl with a 140k state file.

[tillkamppeter]

I have done another test now and the problem does actually not occur with testpappl and it does occur, strangely enough, with the PostScript Printer Application. Strange here is that the loading of the state file is completely done by libpappl and not by the code of the Printer Application, in addition, the line for the installable-options-default is not too long (a printer will need around 70 installable accessories to exceed the allowed 1023 characters here).

[tillkamppeter]

Did you also test with the hp-printer-app?

[tillkamppeter]

testpappl does not use papplMainloop() whereas the PostScript Printer Application (and also hp-printer-app) use it, so it is perhaps a problem of papplMainloop().

[tillkamppeter]

I have found out what the problem is here: While the state file is open for reading (papplSystemLoadState() is running) after each <printer ...> ... </Printer> read and a printer structure created the state file is written (papplSystemSaveState()).

In simple cases this is not a problem (probably when what is written after each printer entry read is identical to what is already there, but with the PostScript Printer Application it is more complex, as first, the <printer ...> line is read and the printer driver data structure is generated independent of the user's default settings (which are not yet read) and after that the defaults, and especially the installable options get read and then the driver data gets updated, which leads to the actual configuration being temporarily different and so the writing of the state file while reading destroys the file.

My suggestion is that the papplSystemLoadState() sets some flag when the state file is open for reading and papplSystemSaveState() simply exits without saving when the flag is set.

[tillkamppeter]

I have now found out what saves the state file already while still loading it in the PostScript Printer Application. I am calling papplSystemSaveState() in the status callback (ps_status()). Now I have changed it into if (papplSystemIsRunning(system)) papplSystemSaveState(system, state_file); and everything works (as the state file is only loaded before starting the system).

[michaelrsweet]

@tillkamppeter Is there a reason why you were manually saving the save vs. letting PAPPL track changes and save when it is safe to do so? Any changes you make to printer objects via papplPrinterSet trigger a state save.

[tillkamppeter]

In the status callback, ps_status() I have the following code:

  if (!extension->updated)
  {
    if ((attr = ippFindAttribute(driver_attrs, "installable-options-default",
                 IPP_TAG_ZERO)) != NULL &&
    ippAttributeString(attr, buf, sizeof(buf)) > 0)
      papplLogPrinter(printer, PAPPL_LOGLEVEL_DEBUG,
              "Applying installable accessories settings: %s", buf);
    else
      papplLogPrinter(printer, PAPPL_LOGLEVEL_DEBUG,
              "Installable Options settings not found");

    // Update the driver data to correspond with the printer hardware
    // accessory configuration ("Installable Options" in the PPD)
    ps_driver_setup(system, NULL, NULL, NULL, &driver_data, &driver_attrs,
            NULL);

    // Copy the vendor option IPP attributes
    vendor_attrs = ippNew();
    for (i = 0; i < driver_data.num_vendor; i ++)
    {
      snprintf(buf, sizeof(buf), "%s-default", driver_data.vendor[i]);
      attr = ippFindAttribute(driver_attrs, buf, IPP_TAG_ZERO);
      if (attr)
    ippCopyAttribute(vendor_attrs, attr, 0);
      snprintf(buf, sizeof(buf), "%s-supported", driver_data.vendor[i]);
      attr = ippFindAttribute(driver_attrs, buf, IPP_TAG_ZERO);
      if (attr)
    ippCopyAttribute(vendor_attrs, attr, 0);
    }

    // Save the updated driver data back to the printer
    papplPrinterSetDriverData(printer, &driver_data, NULL);

    // Save the vendor options IPP attributes back into the driver attributes
    driver_attrs = papplPrinterGetDriverAttributes(printer);
    ippCopyAttributes(driver_attrs, vendor_attrs, 0, NULL, NULL);
    ippDelete(vendor_attrs);

    // Save new default settings (but only if system is running, to not
    // overwrite the state file when it is still loaded during startup)
    if (papplSystemIsRunning(system))
      papplSystemSaveState(system, state_file);
  }

I set extension->updated to false both on initial creation of driver data (for example during load on startup when the driver data is created before the defaults get read) and whenever "Installable Options" settings are submitted from the "Device Settings" web interface page. Then ps_status() runs the ps_driver_setup() function in update mode, adjusting the choice list of each option to the change in the installable options and correcting defaults being a choice which got removed by the change in the installable options.

To get the updated driver data back into the printer's data structure I have to call papplPrinterSetDriverData(). Now the problem of this function is that it does not conserve the driver_attrs IPP structure but re-creates it, losing the vendor option choices and defaults. Therefore I have to save these into a separate IPP structure before and put it back afterwards. And this requires me to manually save state, as if papplPrinterSetDriverData() already saves, this does not contain the IPP attributes of vendor option choices and defaults.

[michaelrsweet]

@tillkamppeter You should be passing vendor_attrs to papplPrinterSetDriverData.

Changing anything returned by papplPrinterGetDriverAttributes is dangerous because there is no concurrency protection.

[tillkamppeter]

Thanks for the hint.

Done.

papplPrinterSetDriverData() does it equivalent to my code but protected with the lock.