From 98dd342e58cf37fb1980662c08dea4e27b6c7e36 Mon Sep 17 00:00:00 2001 From: Ana Bobillo Date: Tue, 20 Oct 2026 04:29:01 +0200 Subject: [PATCH] Fix ATTR-IF-DEFINED/ATTR-IF-NOT-DEFINED handling in ippFileRead. The conditional attribute directives only tested the variable when the attribute callback rejected the name, so without a callback (ipptool) the attribute was always added and with one (ippeveprinter) a rejected name was added when the condition held. Test the variable first, then the callback, and read skipped attributes into the ignored message, which is now allocated when first used. Also reset the current attribute like the ATTR and DEFINE directives do, so a "," after an out-of-band ATTR-IF-* value no longer adds a value to the previous attribute, possibly in a different ipp_t. Add unit tests for both cases to testipp. --- cups/ipp-file.c | 28 ++++---- cups/testipp.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 13 deletions(-) diff --git a/cups/ipp-file.c b/cups/ipp-file.c index c71df9e5e..b5a870d84 100644 --- a/cups/ipp-file.c +++ b/cups/ipp-file.c @@ -544,6 +544,9 @@ ippFileRead(ipp_file_t *file, // I - IPP data file syntax[128], // Attribute syntax (value tag) name[128]; // Attribute name ipp_tag_t value_tag; // Value tag + bool add; // Add this attribute? + + attr = NULL; if (!ippFileReadToken(file, varname, sizeof(varname))) { @@ -572,24 +575,23 @@ ippFileRead(ipp_file_t *file, // I - IPP data file break; } - if (!file->attr_cb || (*file->attr_cb)(file, file->cb_data, name)) + if (!_cups_strcasecmp(token, "ATTR-IF-DEFINED")) + add = ippFileGetVar(file, varname) != NULL; + else // ATTR-IF-NOT-DEFINED + add = ippFileGetVar(file, varname) == NULL; + + if (add && (!file->attr_cb || (*file->attr_cb)(file, file->cb_data, name))) { // Add this attribute... attrs = file->attrs; } - else if (!_cups_strcasecmp(token, "ATTR-IF-DEFINED")) - { - if (ippFileGetVar(file, varname)) - attrs = file->attrs; - else - attrs = ignored; - } - else // ATTR-IF-NOT-DEFINED + else { - if (ippFileGetVar(file, varname)) - attrs = ignored; - else - attrs = file->attrs; + // Ignore this attribute... + if (!ignored) + ignored = ippNew(); + + attrs = ignored; } if (value_tag < IPP_TAG_INTEGER) diff --git a/cups/testipp.c b/cups/testipp.c index 8c80abb48..df6b709ea 100644 --- a/cups/testipp.c +++ b/cups/testipp.c @@ -284,10 +284,13 @@ static ipp_uchar_t mixed[] = // Mixed value buffer // Local functions... // +bool attr_cb(ipp_file_t *f, void *user_data, const char *name); void print_attributes(ipp_t *ipp, int indent); ssize_t read_cb(_ippdata_t *data, ipp_uchar_t *buffer, size_t bytes); +bool read_file_string(const char *s, ipp_fattr_cb_t cb, ipp_t *attrs); ssize_t read_hex(cups_file_t *fp, ipp_uchar_t *buffer, size_t bytes); int test_file(const char *color); +int test_file_attr_if(ipp_fattr_cb_t cb); bool token_cb(ipp_file_t *f, void *user_data, const char *token); ssize_t write_cb(_ippdata_t *data, ipp_uchar_t *buffer, size_t bytes); @@ -1038,6 +1041,8 @@ main(int argc, // I - Number of command-line arguments // Test ippFile API... status |= test_file(/*color*/NULL); status |= test_file("blue"); + status |= test_file_attr_if(/*cb*/NULL); + status |= test_file_attr_if(attr_cb); // Test ippDateToTime and ippTimeToDate testBegin("ippDateToTime(1970/01/02T00:00:00Z)"); @@ -1180,6 +1185,22 @@ main(int argc, // I - Number of command-line arguments } +// +// 'attr_cb()' - Attribute callback for ASCII IPP data file parser. +// + +bool // O - `true` to add, `false` to ignore +attr_cb(ipp_file_t *f, // I - IPP file data + void *user_data, // I - User data pointer + const char *name) // I - Attribute name +{ + (void)f; + (void)user_data; + + return (strncmp(name, "filtered-", 9) != 0); +} + + // // 'print_attributes()' - Print the attributes in a request... // @@ -1240,6 +1261,44 @@ read_cb(_ippdata_t *data, // I - Data } +// +// 'read_file_string()' - Read attributes from an IPP data file string. +// + +bool // O - `true` on success, `false` on error +read_file_string(const char *s, // I - IPP data file contents + ipp_fattr_cb_t cb, // I - Attribute callback or `NULL` + ipp_t *attrs) // I - IPP attributes +{ + bool ret = false; // Return value + cups_file_t *fp; // Temporary file + char filename[1024]; // Temporary filename + ipp_file_t *file; // IPP data file + + + if ((fp = cupsCreateTempFile("testipp", ".test", filename, sizeof(filename))) == NULL) + return (false); + + cupsFilePuts(fp, s); + cupsFileClose(fp); + + if ((file = ippFileNew(/*parent*/NULL, cb, /*error_cb*/NULL, /*cb_data*/NULL)) != NULL) + { + ippFileSetAttributes(file, attrs); + ippFileSetGroupTag(file, IPP_TAG_PRINTER); + + if (ippFileOpen(file, filename, "r")) + ret = ippFileRead(file, token_cb, false); + + ippFileDelete(file); + } + + unlink(filename); + + return (ret); +} + + // // 'read_hex()' - Read a hex dump of an IPP request. // @@ -1490,6 +1549,112 @@ test_file(const char *color) // I - Media color or `NULL` for default } +// +// 'test_file_attr_if()' - Test ATTR-IF-DEFINED and ATTR-IF-NOT-DEFINED. +// + +int // O - Status +test_file_attr_if(ipp_fattr_cb_t cb) // I - Attribute callback or `NULL` +{ + int status = 0; // Status + ipp_t *attrs; // IPP attributes + ipp_attribute_t *attr; // Current attribute + size_t i, // Looping var + count, // Number of values + num_attrs, // Number of attributes + expected; // Expected number of attributes + const char *svalue; // String value + static const char * const vars_test = // Conditional attributes ("yes" = added) + "DEFINE defined yes\n" + "ATTR-IF-DEFINED defined keyword if-defined-defined yes\n" + "ATTR-IF-DEFINED undefined keyword if-defined-undefined no,no\n" + "ATTR-IF-NOT-DEFINED defined keyword if-not-defined-defined no\n" + "ATTR-IF-NOT-DEFINED undefined keyword if-not-defined-undefined yes,yes\n" + "ATTR-IF-DEFINED undefined keyword filtered-if-defined-undefined no\n" + "ATTR-IF-DEFINED defined keyword filtered-if-defined-defined yes\n" + "ATTR-IF-NOT-DEFINED undefined keyword filtered-if-not-defined-undefined yes\n"; + static const char * const comma_test = // Value after out-of-band attribute + "ATTR keyword job-sheets none\n" + "ATTR-IF-NOT-DEFINED undefined unknown job-sheets-col\n" + ",standard\n"; + + + // ATTR-IF-DEFINED and ATTR-IF-NOT-DEFINED only add attributes when the + // variable test passes and the attribute callback accepts the name... + testBegin("ippFileRead(ATTR-IF-DEFINED/ATTR-IF-NOT-DEFINED%s)", cb ? ", attr_cb" : ""); + + attrs = ippNew(); + expected = cb ? 2 : 4; + + if (!read_file_string(vars_test, cb, attrs)) + { + testEndMessage(false, "ippFileRead failed"); + status = 1; + } + else + { + for (attr = ippGetFirstAttribute(attrs), num_attrs = 0; attr; attr = ippGetNextAttribute(attrs), num_attrs ++) + { + for (i = 0, count = ippGetCount(attr); i < count; i ++) + { + if ((svalue = ippGetString(attr, i, NULL)) == NULL || strcmp(svalue, "yes")) + break; + } + + if (i < count) + break; + } + + if (attr) + { + testEndMessage(false, "Unexpected attribute '%s'", ippGetName(attr)); + status = 1; + } + else if (num_attrs != expected) + { + testEndMessage(false, "Got %u attributes, expected %u", (unsigned)num_attrs, (unsigned)expected); + status = 1; + } + else + { + testEndMessage(true, "%u attributes", (unsigned)num_attrs); + } + } + + ippDelete(attrs); + + // A "," after an out-of-band ATTR-IF-DEFINED/ATTR-IF-NOT-DEFINED attribute + // must not add a value to the previous attribute... + testBegin("ippFileRead(',' after out-of-band ATTR-IF-NOT-DEFINED%s)", cb ? ", attr_cb" : ""); + + attrs = ippNew(); + + if (read_file_string(comma_test, cb, attrs)) + { + testEndMessage(false, "ippFileRead succeeded, expected failure"); + status = 1; + } + else if ((attr = ippFindAttribute(attrs, "job-sheets", IPP_TAG_KEYWORD)) == NULL) + { + testEndMessage(false, "missing job-sheets"); + status = 1; + } + else if (ippGetCount(attr) != 1) + { + testEndMessage(false, "Got %u job-sheets values, expected 1", (unsigned)ippGetCount(attr)); + status = 1; + } + else + { + testEnd(true); + } + + ippDelete(attrs); + + return (status); +} + + // // 'token_cb()' - Token callback for ASCII IPP data file parser. //