Skip to content

fix: preserve embedded TIFF profiles on parse failure - #2188

Merged
xsscx merged 1 commit into
masterfrom
ci-qa-fix-regression-800ac41-tiff-read
Aug 17, 2026
Merged

fix: preserve embedded TIFF profiles on parse failure#2188
xsscx merged 1 commit into
masterfrom
ci-qa-fix-regression-800ac41-tiff-read

Conversation

@xsscx

@xsscx xsscx commented Aug 17, 2026

Copy link
Copy Markdown
Member

PR Summary

#2187

Checklist

  • Signed all Commits in PR
  • Built locally according to docs/build.md
  • Followed the guidelines in Contributing document
  • Ran relevant CTest/profile tests from docs/ctest.md
  • Updated documentation for user-visible behavior changes
  • Ran sanitizer coverage for memory-safety or parser changes
  • Added or updated regression coverage for behavior changes
  • For Python package changes, followed docs/python-packaging-release.md for PR and merge requirements
  • Did not change maintainer-owned workflow, CTest, CPack, sanitizer, release, or security infrastructure unless requested by an iccDEV maintainer
  • New source files include the ICC copyright and BSD 3-Clause license header
  • Code style matches nearby code: 2-space indent, K&R braces, m_ members

Legal Requirements

All official software projects hosted by the International Color Consoritum (ICC)
follows the open source software best practice policies. The International Color Consoritum IP policy governs ICC specification development and contributions to ICC open source software. Software contributions are also covered by the Contributor License Agreement (CLA).

Contributor License Agreements

Developers who wish to contribute code to be considered for inclusion
in ICC software must first complete a Contributor License Agreement
(CLA)
.

There is no cost or membership requirement to sign the ICC Contributor License Agreement (CLA). Please note that this is different from membership in the International Color Consortium (ICC). If your organization relies on our projects, please become a member. Membership dues are an essential source of funding and investment for these projects.

  • If you are an individual writing the code on your own time and you are SURE you are the sole owner of any intellectual property you contribute, you can sign the CLA as an individual contributor.

  • If you are writing the code as part of your job, or if there is any possibility that your employer might think they own any intellectual property you create, then you should use the Corporate Contributor Licence Agreement

License

ICC software is licensed under the BSD 3-Clause "New" or "Revised" License. Contributions to ICC software projects should abide by that license unless otherwised specified or approved by the ICC.

Copyright Notices

All new source files must begin with the ICC Copyright notice and include or reference the BSD 3-Clause "New" or "Revised" License.

INTELLECTUAL PROPERTY & PATENTS

Participation in ICC's development activities is subject to ICC's Patent Policy.

Maintainer Review Required

If you have questions, contact a listed Maintainer.

@xsscx xsscx self-assigned this Aug 17, 2026
@xsscx
xsscx requested a lite review from Copilot August 17, 2026 16:23
@github-actions github-actions Bot added Tools Command-line tool or GUI tool changes Source C or C++ source code changes Scripts Shell, PowerShell, or repository automation scripts Unix Linux, macOS, Bash, or POSIX shell scope pending CI checks still running labels Aug 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates iccTiffDump to always extract the embedded TIFF ICC profile bytes as-is (before parsing/validation), so malformed or nonconformant profiles are preserved for diagnostics even when the tool exits with an error. It also adds a recursion-depth bound for full tag loading to mitigate pathological nested embedded-profile inputs (Issue #2187).

Changes:

  • Write embedded ICC bytes byte-for-byte to the requested output path before parsing/validation, using a temp-file + atomic rename flow.
  • Add a thread-local recursion depth guard to CIccProfile::ReadTags() to bound recursive embedded-profile tag expansion.
  • Update hardening regression coverage and tool documentation to reflect the new “preserve on failure” extraction semantics.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
Tools/CmdLine/IccTiffDump/Readme.md Documents byte-for-byte extraction and atomic output behavior.
Tools/CmdLine/IccTiffDump/iccTiffDump.cpp Implements raw extraction prior to validation and switches export behavior accordingly.
IccProfLib/IccProfile.cpp Adds a recursion depth limit for recursive full tag loading (ReadTags).
.github/scripts/iccdev-tiffdump-output-hardening-tests.sh Extends regression coverage to assert preservation and bounded recursion behaviors.
Suppressed comments (1)

.github/scripts/iccdev-tiffdump-output-hardening-tests.sh:340

  • Similar to the iccTiffDump bounded-recursion test above: timeout returns 124 on timeout, which currently satisfies the 1..127 exit-code assertion. If iccToXml hangs and timeout kills it after producing partial output, this check could incorrectly pass. Make status==124 fail explicitly.
  timeout 10 "$TOXML" "$nested" "$xml" > "$log" 2>&1 || status=$?
  [ "$status" -ge 1 ] && [ "$status" -le 127 ] || return 1

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/scripts/iccdev-tiffdump-output-hardening-tests.sh
@xsscx

xsscx commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

PR Review Status

2026-08-17 16:29:49 UTC

  1. Valid
  2. Actionable
  3. Fix
  4. Test
  5. Push

@xsscx xsscx added Copilot Copilot use indicated by Maintainer Codex Codex use indicated by Maintainer labels Aug 17, 2026
@xsscx
xsscx force-pushed the ci-qa-fix-regression-800ac41-tiff-read branch from 5366357 to 0127b4c Compare August 17, 2026 16:31
@xsscx
xsscx requested a lite review from Copilot August 17, 2026 16:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@xsscx

xsscx commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

PR Review Status

2026-08-17 16:38:30 UTC

Copilot PR Review Documentation

  1. PR should be added to the Build Graph for Surface Suppressed Comments
  2. Both Review Items are Actionable
  3. Set Criteria & Catagory to Blocking

Keywords: PR Review Copilot Graph

@xsscx

xsscx commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

Automated Review Blind Spot(s)

  1. Note that Copilot is Disconnected from CI, SEE BELOW
  2. Logic will need to delay Copilot PR Review until CI Complete
  3. iccBot on Fail throw no Copilot PR Review
  4. iccBot on Success throw ok Copilot PR Review
  5. Label interval TBD (15m)

CI Report

2026-08-17 16:42:31 UTC

Job

132/177 Test #140: iccdev.issue-1150-output-failure-regression ....................***Failed    0.45 sec
TIFFReadDirectory: Warning, Sum of Photometric type-related color channels and ExtraSamples doesn't match SamplesPerPixel. Defining non-color channels as ExtraSamples..
-------------------->Tiff Image Dump<---------------------------
Filename:          /__w/iccDEV/iccDEV/Testing/hybrid/Data/smCows380_5_780.tif
Size:              (600 x 420) pixels, (8.33" x 5.83")
Planar:            Interleaved samples
BitsPerSample:     16 (unsigned integer)
SamplesPerPixel:   81
Photometric:       Min Is Black
BytesPerLine:      97200
Resolution:        (72.000000 x 72.000000) pixels per/inch
Compression:       LZW
Profile:           Embedded

Profile extracted byte-for-byte to: /dev/full
 Version:          5.00
 Class:            ColorSpace
 Color Space:      0x0051ChannelData
 Spectral PCS:     0x0051ChannelReflectanceData
 Spectral Range:   start=380.0nm, end=780.0nm, steps=81
 Description:      380nm to 780nm in 5nm steps spectral reflectance using D50 and standard 2deg observer with Wpt MAT based PCC
  [FAIL] issue-1150-output-failure -- tiffdump-dev-full reported success for a failed output path

@xsscx
xsscx force-pushed the ci-qa-fix-regression-800ac41-tiff-read branch from 0127b4c to 52a6f9c Compare August 17, 2026 16:50
@xsscx

xsscx commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

PR Status

2026-08-17 17:07:39 UTC

Windows 11 Repro

  • Issue was [FAIL] issue-1150-output-failure -- tiffdump-dev-full reported success for a failed output path

Build & Generate Profiles

git clone https://github.com/InternationalColorConsortium/iccDEV.git
cd iccDEV
git fetch origin pull/2188/head:pr-2188
git checkout pr-2188
vcpkg integrate install
vcpkg install
cmake --preset vs2022-x64 -B msvc -S Build/Cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
cmake --build msvc -- /m /maxcpucount
          $exeDirs = Get-ChildItem -Recurse -File -Include *.exe -Path msvc |
              Where-Object { $_.FullName -match 'icc' -and $_.FullName -notmatch '\\CMakeFiles\\' -and $_.Name -notmatch '^CMake(C|CXX)CompilerId\.exe$' } |
              ForEach-Object { Split-Path $_.FullName -Parent } |
              Sort-Object -Unique
          $env:PATH = ($exeDirs -join ';') + ';' + $env:PATH
          $env:PATH -split ';' | Select-String "icc"
          $toolDirs = Get-ChildItem -Recurse -File -Include *.exe -Path .\Tools\ | ForEach-Object { Split-Path -Parent $_.FullName } | Sort-Object -Unique
          $env:PATH = ($toolDirs -join ';') + ';' + $env:PATH
          $env:PATH -split ';'
          pwd
          cd Testing
         .\CreateAllProfiles.bat
          .\RunTests.bat
          cd CalcTest\
          .\checkInvalidProfiles.bat
          .\runtests.bat
          cd ..\Display
          .\RunProtoTests.bat
          cd ..\HDR
          .\mkprofiles.bat
          cd ..\mcs\
          .\updateprev.bat
          .\updateprevWithBkgd.bat
          cd ..\Overprint
          .\RunTests.bat
          cd ..
          cd hybrid
          .\BuildAndTest.bat
          cd ..
          pwd
wget https://github.com/xsscx/fuzz/raw/refs/heads/master/graphics/tif/nested-embedded-depth-512.tif
iccTiffDump nested-embedded-depth-512.tif foo.bar
          # Collect .icc profile information
          $profiles = Get-ChildItem -Path . -Filter "*.icc" -Recurse -File
          $totalCount = $profiles.Count
          
          # Group profiles by directory
          $groupedProfiles = $profiles | Group-Object { $_.Directory.FullName }
          
          # Generate Summary Report
          Write-Host "`n========================="
          Write-Host " ICC Profile Report"
          Write-Host "========================="
          
          # Print count per subdirectory
          foreach ($group in $groupedProfiles) {
              Write-Host ("{0}: {1} .icc profiles" -f $group.Name, $group.Count)
          }
          
          Write-Host "`nTotal .icc profiles found: $totalCount"
          Write-Host "=========================`n"
          
          Write-Host "All Done!"

QA Test Results

Success for Linux, macOS & Windows

@xsscx

xsscx commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

CI Report

2026-08-17 17:10:41 UTC

ci-regression-checks

Job

 99/188 Test  #99: iccdev.tool-coverage ...........................................***Failed   66.14 sec

@xsscx
xsscx force-pushed the ci-qa-fix-regression-800ac41-tiff-read branch from 52a6f9c to 954cfe7 Compare August 17, 2026 17:17
@xsscx

xsscx commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

Unix Repro

2026-08-17 17:43:04 UTC

git clone https://github.com/InternationalColorConsortium/iccDEV.git
cd iccDEV
git fetch origin pull/2188/head:pr-2188
git checkout pr-2188
cd Build
git branch && echo "---" && git log --oneline -1
CC=clang CXX=clang++ CXXFLAGS="-fsanitize=address,undefined,integer,bounds,null,float-divide-by-zero,alignment,vla-bound -fno-omit-frame-pointer -g -O0" LDFLAGS="-fsanitize=address,undefined,integer,bounds,null,float-divide-by-zero,alignment,vla-bound" cmake Cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_ASAN=ON -DENABLE_UBSAN=ON -DENABLE_TOOLS=ON -Wl,--build-id
make -j32
        cd ../Testing/
        echo "=== Updating PATH ==="
         for d in ../Build/Tools/*; do
          [ -d "$d" ] && export PATH="$(realpath "$d"):$PATH"
         done
          printf '%s\n%s\n%s\n' 'unsigned-integer-overflow:*/IccMD5.cpp' 'shift-base:*/IccMD5.cpp' 'shift-exponent:*/IccMD5.cpp' > silence.txt && ASAN_OPTIONS='print_scariness=1:halt_on_error=1:detect_leaks=0' UBSAN_OPTIONS='halt_on_error=0:suppressions='"$PWD"'/silence.txt' ./CreateAllProfiles.sh
          printf '%s\n%s\n%s\n' 'unsigned-integer-overflow:*/IccMD5.cpp' 'shift-base:*/IccMD5.cpp' 'shift-exponent:*/IccMD5.cpp' > silence.txt && ASAN_OPTIONS='print_scariness=1:halt_on_error=1:detect_leaks=0' UBSAN_OPTIONS='halt_on_error=0:suppressions='"$PWD"'/silence.txt' ./RunTests.sh
          cd HDR
          printf '%s\n%s\n%s\n' 'unsigned-integer-overflow:*/IccMD5.cpp' 'shift-base:*/IccMD5.cpp' 'shift-exponent:*/IccMD5.cpp' > silence.txt && ASAN_OPTIONS='print_scariness=1:halt_on_error=1:detect_leaks=0' UBSAN_OPTIONS='halt_on_error=0:suppressions='"$PWD"'/silence.txt' ./mkprofiles.sh
          cd ..
          cd Display
          printf '%s\n%s\n%s\n' 'unsigned-integer-overflow:*/IccMD5.cpp' 'shift-base:*/IccMD5.cpp' 'shift-exponent:*/IccMD5.cpp' > silence.txt && ASAN_OPTIONS='print_scariness=1:halt_on_error=1:detect_leaks=0' UBSAN_OPTIONS='halt_on_error=0:suppressions='"$PWD"'/silence.txt' ./RunProtoTests.sh
          cd ..
          cd hybrid
          printf '%s\n%s\n%s\n' 'unsigned-integer-overflow:*/IccMD5.cpp' 'shift-base:*/IccMD5.cpp' 'shift-exponent:*/IccMD5.cpp' > silence.txt && ASAN_OPTIONS='print_scariness=1:halt_on_error=1:detect_leaks=0' UBSAN_OPTIONS='halt_on_error=0:suppressions='"$PWD"'/silence.txt' ./BuildAndTest.sh
          cd ..
          cd CalcTest
          printf '%s\n%s\n%s\n' 'unsigned-integer-overflow:*/IccMD5.cpp' 'shift-base:*/IccMD5.cpp' 'shift-exponent:*/IccMD5.cpp' > silence.txt && ASAN_OPTIONS='print_scariness=1:halt_on_error=1:detect_leaks=0' UBSAN_OPTIONS='halt_on_error=0:suppressions='"$PWD"'/silence.txt' ./checkInvalidProfiles.sh
          cd ..
          cd mcs
          printf '%s\n%s\n%s\n' 'unsigned-integer-overflow:*/IccMD5.cpp' 'shift-base:*/IccMD5.cpp' 'shift-exponent:*/IccMD5.cpp' > silence.txt && ASAN_OPTIONS='print_scariness=1:halt_on_error=1:detect_leaks=0' UBSAN_OPTIONS='halt_on_error=0:suppressions='"$PWD"'/silence.txt' ./updateprev.sh
          printf '%s\n%s\n%s\n' 'unsigned-integer-overflow:*/IccMD5.cpp' 'shift-base:*/IccMD5.cpp' 'shift-exponent:*/IccMD5.cpp' > silence.txt && ASAN_OPTIONS='print_scariness=1:halt_on_error=1:detect_leaks=0' UBSAN_OPTIONS='halt_on_error=0:suppressions='"$PWD"'/silence.txt' ./updateprevWithBkgd.sh
          cd ..
wget https://github.com/xsscx/fuzz/raw/refs/heads/master/graphics/tif/nested-embedded-depth-512.tif
iccTiffDump nested-embedded-depth-512.tif foo.bar

Expected Output

2026-08-17 13:50:05 (6.37 MB/s) - ‘nested-embedded-depth-512.tif’ saved [102744/102744]

-------------------->Tiff Image Dump<---------------------------
Filename:          nested-embedded-depth-512.tif
Size:              (1 x 1) pixels, (0.01" x 0.01")
Planar:            Interleaved samples
BitsPerSample:     8 (unsigned integer)
SamplesPerPixel:   3
Photometric:       RGB
BytesPerLine:      3
Resolution:        (72.000000 x 72.000000) pixels per/inch
Compression:       None
Profile:           Embedded

Profile extracted byte-for-byte to: foo.bar
 Version:          5.00
 Class:            Display
 Color Space:      RgbData
 Colorimetric PCS: XYZData
 Sub-Profile:      Embedded
  Version:          5.00
  Class:            Display
  Color Space:      RgbData
  Colorimetric PCS: XYZData
  Sub-Profile:      Embedded
   Version:          5.00
   Class:            Display
   Color Space:      RgbData
   Colorimetric PCS: XYZData
   Sub-Profile:      Embedded
    Version:          5.00
    Class:            Display
    Color Space:      RgbData
    Colorimetric PCS: XYZData
    Sub-Profile:      Embedded
     Subprofile recursion halted

Unable to read embedded ICC profile

@xsscx

xsscx commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

Pre Merge Report

2026-08-17 17:58:15 UTC

TODO Summary

  1. CTest run in ci-pr-action do not equal ci-regression-checks
  2. PR indicates Merge Ready whereas we do not want this yet to Merge because ci-regression-checks did in fact Fail
  3. Unify & Align re-usable workflow stub so its included in ci-pr-action
  4. Resolve the Stub Divergence

@xsscx
xsscx merged commit 3e34820 into master Aug 17, 2026
20 of 21 checks passed
@xsscx
xsscx deleted the ci-qa-fix-regression-800ac41-tiff-read branch August 17, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Codex Codex use indicated by Maintainer Copilot Copilot use indicated by Maintainer pending CI checks still running Scripts Shell, PowerShell, or repository automation scripts Source C or C++ source code changes Tools Command-line tool or GUI tool changes Unix Linux, macOS, Bash, or POSIX shell scope

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bisect: 800ac41 TIFF Read regression for recursive tag loading & validation

2 participants