From d0eef28ee12ad1b25a89c9fe0b4a90f83830d298 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Fri, 17 Aug 2012 15:30:29 -0500 Subject: [PATCH] [libpng16] Added "tunknown" test and corrected a logic error in png_handle_unknown() when SAVE support is absent. Moved the shell test scripts for contrib/libtests from the libpng top directory to contrib/libtests. png_handle_unknown() must always read or skip the chunk, if SAVE_UNKNOWN_CHUNKS is turned off *and* the application does not set a user callback an unknown chunk will not be read, leading to a read error, which was revealed by the "tunknown" test. --- ANNOUNCE | 7 ++ CHANGES | 7 ++ configure.ac | 2 +- contrib/libtests/gentests.sh | 0 .../libtests/test-pngstest.sh | 0 .../libtests/test-pngvalid-full.sh | 0 .../libtests/test-pngvalid-simple.sh | 0 contrib/libtests/test-unknown.c | 102 ++++++++++++------ contrib/libtests/test-unknown.sh | 38 +++++++ pngrutil.c | 18 +++- 10 files changed, 138 insertions(+), 36 deletions(-) mode change 100644 => 100755 contrib/libtests/gentests.sh rename test-pngstest.sh => contrib/libtests/test-pngstest.sh (100%) rename test-pngvalid-full.sh => contrib/libtests/test-pngvalid-full.sh (100%) rename test-pngvalid-simple.sh => contrib/libtests/test-pngvalid-simple.sh (100%) create mode 100755 contrib/libtests/test-unknown.sh diff --git a/ANNOUNCE b/ANNOUNCE index f89d574e1..682f837a8 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -462,6 +462,13 @@ Version 1.6.0beta28 [August 17, 2012] unsafe-to-copy chunks which were dropped before) and eliminates the repositioning of vpAg and sTER in pngtest.png by changing pngtest.png (so the chunks are where libpng would put them). + Added "tunknown" test and corrected a logic error in png_handle_unknown() + when SAVE support is absent. Moved the shell test scripts for + contrib/libtests from the libpng top directory to contrib/libtests. + png_handle_unknown() must always read or skip the chunk, if + SAVE_UNKNOWN_CHUNKS is turned off *and* the application does not set + a user callback an unknown chunk will not be read, leading to a read + error, which was revealed by the "tunknown" test. Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/CHANGES b/CHANGES index c8a8c944a..52c3e38aa 100644 --- a/CHANGES +++ b/CHANGES @@ -4213,6 +4213,13 @@ Version 1.6.0beta28 [August 17, 2012] unsafe-to-copy chunks which were dropped before) and eliminates the repositioning of vpAg and sTER in pngtest.png by changing pngtest.png (so the chunks are where libpng would put them). + Added "tunknown" test and corrected a logic error in png_handle_unknown() + when SAVE support is absent. Moved the shell test scripts for + contrib/libtests from the libpng top directory to contrib/libtests. + png_handle_unknown() must always read or skip the chunk, if + SAVE_UNKNOWN_CHUNKS is turned off *and* the application does not set + a user callback an unknown chunk will not be read, leading to a read + error, which was revealed by the "tunknown" test. Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/configure.ac b/configure.ac index 19ea12c4b..7967d8b61 100644 --- a/configure.ac +++ b/configure.ac @@ -25,7 +25,7 @@ AC_CONFIG_MACRO_DIR([scripts]) # color-tests requires automake 1.11 or later # silent-rules requires automake 1.11 or later # dist-xz requires automake 1.11 or later -# 1.11.1 fixes a security issue in 1.11 +# 1.12.2 fixes a security issue in 1.11.2 and 1.12.1 AM_INIT_AUTOMAKE([1.12.2 foreign dist-xz dist-bzip2 color-tests silent-rules]) # was: # AM_INIT_AUTOMAKE diff --git a/contrib/libtests/gentests.sh b/contrib/libtests/gentests.sh old mode 100644 new mode 100755 diff --git a/test-pngstest.sh b/contrib/libtests/test-pngstest.sh similarity index 100% rename from test-pngstest.sh rename to contrib/libtests/test-pngstest.sh diff --git a/test-pngvalid-full.sh b/contrib/libtests/test-pngvalid-full.sh similarity index 100% rename from test-pngvalid-full.sh rename to contrib/libtests/test-pngvalid-full.sh diff --git a/test-pngvalid-simple.sh b/contrib/libtests/test-pngvalid-simple.sh similarity index 100% rename from test-pngvalid-simple.sh rename to contrib/libtests/test-pngvalid-simple.sh diff --git a/contrib/libtests/test-unknown.c b/contrib/libtests/test-unknown.c index 34871315e..3dc4da50a 100644 --- a/contrib/libtests/test-unknown.c +++ b/contrib/libtests/test-unknown.c @@ -11,12 +11,9 @@ * * NOTES: * This is a C program that is intended to be linked against libpng. It - * generates bitmaps internally, stores them as PNG files (using the - * sequential write code) then reads them back (using the sequential - * read code) and validates that the result has the correct data. - * - * The program can be modified and extended to test the correctness of - * transformations performed by libpng. + * allows the libpng unknown handling code to be tested by interpreting + * arguemnts to save or discard combinations of chunks. The program is + * currently just a minimal validation for the built-in libpng facilities. */ #include @@ -25,6 +22,8 @@ #include +#ifdef PNG_READ_SUPPORTED + #if PNG_LIBPNG_VER < 10500 /* This deliberately lacks the PNG_CONST. */ typedef png_byte *png_const_bytep; @@ -365,11 +364,20 @@ check(FILE *fp, int argc, const char **argv, png_uint_32p flags/*out*/) if (chunk >= 0) { - png_byte name[5]; + /* These #if tests have the effect of skipping the arguments + * if SAVE support is unavailable - we can't do a useful test + * in this case, so we just check the arguments! This could + * be improved in the future by using the read callback. + */ +# ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED + png_byte name[5]; - memcpy(name, chunk_info[chunk].name, 5); - png_set_keep_unknown_chunks(png_ptr, option, name, 1); - chunk_info[chunk].keep = option; + memcpy(name, chunk_info[chunk].name, 5); + png_set_keep_unknown_chunks(png_ptr, option, name, 1); + chunk_info[chunk].keep = option; +# else + (void)option; +# endif continue; } @@ -378,8 +386,10 @@ check(FILE *fp, int argc, const char **argv, png_uint_32p flags/*out*/) case 7: /* default */ if (memcmp(argv[i], "default", 7) == 0) { - png_set_keep_unknown_chunks(png_ptr, option, NULL, 0); - def = option; +# ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED + png_set_keep_unknown_chunks(png_ptr, option, NULL, 0); + def = option; +# endif continue; } @@ -388,15 +398,21 @@ check(FILE *fp, int argc, const char **argv, png_uint_32p flags/*out*/) case 3: /* all */ if (memcmp(argv[i], "all", 3) == 0) { - png_set_keep_unknown_chunks(png_ptr, option, NULL, -1); - def = option; +# ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED + png_set_keep_unknown_chunks(png_ptr, option, NULL, -1); + def = option; - for (chunk = 0; chunk < NINFO; ++chunk) - if (chunk_info[chunk].all) - chunk_info[chunk].keep = option; + for (chunk = 0; chunk < NINFO; ++chunk) + if (chunk_info[chunk].all) + chunk_info[chunk].keep = option; +# endif continue; } + break; + + default: /* some misplaced = */ + break; } } @@ -499,7 +515,7 @@ check_handling(const char *file, int def, png_uint_32 chunks, png_uint_32 known, int i = find_by_flag(flag); int keep = chunk_info[i].keep; const char *type; - const char *error = NULL; + const char *errorx = NULL; if (chunk_info[i].unknown) { @@ -513,34 +529,38 @@ check_handling(const char *file, int def, png_uint_32 chunks, png_uint_32 known, type = "UNKNOWN (specified)"; if (flag & known) - error = "chunk processed"; + errorx = "chunk processed"; else switch (keep) { case PNG_HANDLE_CHUNK_AS_DEFAULT: if (flag & unknown) - error = "DEFAULT: unknown chunk saved"; + errorx = "DEFAULT: unknown chunk saved"; break; case PNG_HANDLE_CHUNK_NEVER: if (flag & unknown) - error = "DISCARD: unknown chunk saved"; + errorx = "DISCARD: unknown chunk saved"; break; case PNG_HANDLE_CHUNK_IF_SAFE: if (ancillary(chunk_info[i].name)) { if (!(flag & unknown)) - error = "IF-SAFE: unknown ancillary chunk lost"; + errorx = "IF-SAFE: unknown ancillary chunk lost"; } else if (flag & unknown) - error = "IF-SAFE: unknown critical chunk saved"; + errorx = "IF-SAFE: unknown critical chunk saved"; break; case PNG_HANDLE_CHUNK_ALWAYS: if (!(flag & unknown)) - error = "SAVE: unknown chunk lost"; + errorx = "SAVE: unknown chunk lost"; + break; + + default: + errorx = "internal error: bad keep"; break; } } /* unknown chunk */ @@ -555,43 +575,47 @@ check_handling(const char *file, int def, png_uint_32 chunks, png_uint_32 known, * caught below when checking for inconsistent processing. */ if (keep != PNG_HANDLE_CHUNK_AS_DEFAULT) - error = "!DEFAULT: known chunk processed"; + errorx = "!DEFAULT: known chunk processed"; } else /* not processed */ switch (keep) { case PNG_HANDLE_CHUNK_AS_DEFAULT: - error = "DEFAULT: known chunk not processed"; + errorx = "DEFAULT: known chunk not processed"; break; case PNG_HANDLE_CHUNK_NEVER: if (flag & unknown) - error = "DISCARD: known chunk saved"; + errorx = "DISCARD: known chunk saved"; break; case PNG_HANDLE_CHUNK_IF_SAFE: if (ancillary(chunk_info[i].name)) { if (!(flag & unknown)) - error = "IF-SAFE: known ancillary chunk lost"; + errorx = "IF-SAFE: known ancillary chunk lost"; } else if (flag & unknown) - error = "IF-SAFE: known critical chunk saved"; + errorx = "IF-SAFE: known critical chunk saved"; break; case PNG_HANDLE_CHUNK_ALWAYS: if (!(flag & unknown)) - error = "SAVE: known chunk lost"; + errorx = "SAVE: known chunk lost"; + break; + + default: + errorx = "internal error: bad keep (2)"; break; } } - if (error != NULL) + if (errorx != NULL) { ++error_count; fprintf(stderr, "%s: %s %s %s: %s\n", - file, type, chunk_info[i].name, position, error); + file, type, chunk_info[i].name, position, errorx); } chunks &= ~flag; @@ -622,6 +646,10 @@ main(int argc, const char **argv) exit(2); } +# ifndef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED + fprintf(stderr, + "test-unknown: warning: no 'save' support so arguments ignored\n"); +# endif fp = fopen(argv[argc], "rb"); if (fp == NULL) { @@ -681,3 +709,13 @@ main(int argc, const char **argv) return error_count + (strict ? warning_count : 0); } + +#else +int +main(void) +{ + fprintf(stderr, + " test ignored because libpng was not built with unknown chunk support\n"); + return 0; +} +#endif diff --git a/contrib/libtests/test-unknown.sh b/contrib/libtests/test-unknown.sh new file mode 100755 index 000000000..163f5b0d9 --- /dev/null +++ b/contrib/libtests/test-unknown.sh @@ -0,0 +1,38 @@ +#!/bin/sh +# +# Run the unknown API tests +err=0 +image="${srcdir}/pngtest.png" +# +# stream 4 is used for the output of the shell, pngtest-log.txt gets all the +# normal program output. +exec 4>&1 1>>pngtest-log.txt 2>&1 + +echo +echo "============ test-unknown.sh ==============" + +echo "Running test-unknown.sh" >&4 + +for tests in \ + "discard default=discard"\ + "save default=save"\ + "if-safe default=if-safe"\ + "vpAg vpAg=if-safe"\ + "sTER sTER=if-safe"\ + "IDAT default=discard IDAT=save"\ + "sAPI bKGD=save cHRM=save gAMA=save all=discard iCCP=save sBIT=save sRGB=save" +do + set $tests + test="$1" + shift + + if ./tunknown "$@" "$image" 4>&- + then + echo " PASS: test-unknown $test" >&4 + else + echo " FAIL: test-unknown $test" >&4 + err=1 + fi +done + +exit $err diff --git a/pngrutil.c b/pngrutil.c index 88758deeb..975b3910f 100644 --- a/pngrutil.c +++ b/pngrutil.c @@ -2883,9 +2883,8 @@ png_handle_unknown(png_structrp png_ptr, png_inforp info_ptr, keep = PNG_HANDLE_CHUNK_NEVER; /* insufficient memory */ } -# ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED - else -# endif + else + /* Use the SAVE_UNKNOWN_CHUNKS code or skip the chunk */ # endif /* PNG_READ_USER_CHUNKS_SUPPORTED */ # ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED @@ -2913,6 +2912,19 @@ png_handle_unknown(png_structrp png_ptr, png_inforp info_ptr, # ifndef PNG_READ_USER_CHUNKS_SUPPORTED # error no method to support READ_UNKNOWN_CHUNKS # endif + + { + /* If here there is no read callback pointer set and no support is + * compiled in to just save the unknown chunks, so simply skip this + * chunk. If 'keep' is something other than AS_DEFAULT or NEVER then + * the app has erroneously asked for unknown chunk saving when there + * is no support. + */ + if (keep > PNG_HANDLE_CHUNK_NEVER) + png_app_error(png_ptr, "no unknown chunk support available"); + + png_crc_finish(png_ptr, length); + } # endif # ifdef PNG_STORE_UNKNOWN_CHUNKS_SUPPORTED