Closes #509: Prevent duplicate staticContent blocks causing 500s on IIS by Honemo · Pull Request #1181 · wp-media/imagify-plugin

Honemo · 2026-07-13T10:03:28Z

AI-generated — created by an automated pipeline. Review before acting on this.

Closes #509

Description

Fixes #509

On IIS/Windows servers Imagify wrote a whole <staticContent> container element into web.config for both the WebP and AVIF MIME mappings, never checking whether one already existed. IIS allows exactly one <staticContent> collection under system.webServer; a second sibling (created by Imagify's own WebP+AVIF classes, or colliding with a foreign one) makes IIS fail to parse web.config and return HTTP 500 for the entire site. This has caused repeated live-site outages since 2020.

This PR:

Retargets Webp\IIS/Avif\IIS to emit only a leaf <mimeMap> fragment against the single shared /configuration/system.webServer/staticContent node, letting the existing get_node()/prepend_node() machinery create/merge the container instead of prepending a new one every time.
Adds a fileExtension-based dedupe in AbstractIISDirConfFile::insert_contents() so Imagify's own .webp/.avif mimeMap is identified and replaced idempotently (on both add() and remove() paths), guaranteeing a single <staticContent> with no duplicate fileExtension keys.
Adds a one-time, version-gated self-heal migration (inc/admin/upgrader.php::_imagify_new_upgrade(), 2.3.1 block) that collapses already-broken production installs (duplicate Imagify-created <staticContent> siblings) via a remove-then-add sequence, guarded by a positive if ( ! empty( $is_iis7 ) ) wrapper (never an early return) so it never blocks sibling migration blocks, and never fatals on WP_Error.

Type of change

Bug fix (non-breaking change which fixes an issue).

Detailed scenario

What was tested

Automated (all scenarios below are covered by new/updated PHPUnit tests, run against real DOMDocument + temp web.config files in Tests/Integration/classes/WriteFile/AbstractIISDirConfFile/):

Fresh web.config → single <staticContent> created with the correct mimeMap.
<system.webServer> present with no <staticContent> at all → container auto-created.
Foreign <staticContent> already present → Imagify's mimeMap merges in; foreign mimeMaps preserved; exactly one <staticContent> remains.
Both WebP and AVIF add()'d → both mimeMaps land inside the SAME <staticContent>, never two blocks.
Idempotent double add() → no duplication, count stays 1.
Foreign <staticContent> already containing a .webp mimeMap → replaced (accepted tradeoff, documented in spec), no duplicate fileExtension key.
remove() after merge into a foreign block → foreign block + its mimeMaps survive; only Imagify's mimeMap is removed.
Malformed web.config (DOMDocument::load failure) → WP_Error('not_read'), file left untouched.
Self-heal migration: two pre-existing Imagify-created <staticContent> siblings (the reported broken state) + display_nextgen on → collapses to ONE <staticContent> containing BOTH .webp and .avif (verifies the corrected non-XOR gating against optimization_format).
Self-heal migration with a foreign <staticContent> also present → foreign block preserved, Imagify duplicates collapsed into it.
Self-heal migration with display_nextgen off → remove-only, no re-add, no fatal.
Self-heal migration on non-IIS ($is_iis7 unset) → web.config untouched (guard-scope regression check for the positive-conditional MUST_HAVE).
Self-heal migration with conf edition disabled (unwritable) → skips gracefully, no fatal, file untouched.
Partial-failure scenario (webp add succeeds, avif add forced to fail mid-sequence) → degraded-but-valid state (one <staticContent>, .webp present, .avif absent), no fatal, WP_Error returned cleanly.

Unit tests (Tests/Unit/classes/Webp/IIS/GetRawNewContentsTest.php, Tests/Unit/classes/Avif/IIS/GetRawNewContentsTest.php) assert the emitted fragment string uses the new @parent target and a bare leaf <mimeMap> with no wrapping <staticContent> and no invented name attribute.

Full suite run: composer run-tests → 266 unit tests / 554 assertions, 0 failures; 77 integration tests / 239 assertions, 0 failures (2 pre-existing risky tests unrelated to this change, in GetNextgenCoverage — missing DB table in local test env).

How to test

On a local/staging IIS site (or by seeding a temp web.config per the new test fixtures), enable "Display next-gen format" for both WebP and AVIF.
Inspect web.config: confirm exactly one <staticContent> element exists under system.webServer, containing both <mimeMap fileExtension=".webp" .../> and <mimeMap fileExtension=".avif" .../>.
Toggle the setting off, confirm the mimeMaps are removed and no <staticContent> duplication occurs.
To validate the self-heal migration: manually seed a web.config with two Imagify-created <staticContent> siblings (simulating a broken pre-fix install), bump IMAGIFY_VERSION/trigger imagify_upgrade, and confirm the file collapses to one <staticContent>.
Run composer run-tests for the automated coverage above.

Real IIS-server validation (does IIS actually accept the fixed web.config) is arranged separately by the reporting user before final merge, per the approved spec's user-locked decision — DOM-level integration tests are accepted as sufficient for this PR.

Affected Features & Quality Assurance Scope

WebP/AVIF "Display next-gen format" feature on IIS/Windows hosting (classes/Webp/IIS.php, classes/Avif/IIS.php, classes/WriteFile/AbstractIISDirConfFile.php).
Plugin upgrade routine (inc/admin/upgrader.php) — one new version-gated block, IIS-only, no effect on Apache/nginx.
Out of scope (tracked separately as IIS: duplicate <preConditions> singleton collision in WebP/AVIF rewrite-rules classes #1180): the <preConditions> singleton collision in Webp|Avif/RewriteRules/IIS.php.

Technical description

Documentation

No public API surface changed (no new hooks, options, REST routes, or capabilities) — get_owned_mime_extensions() is a new protected internal method used only by the IIS writer classes themselves. The docs skill was run and returned SKIP for this reason.

New dependencies

None.

Risks

High risk area (live-site-outage bug, cannot validate against a real IIS server in CI). Mitigated by: root-cause fix that structurally prevents future duplicate <staticContent> collections; a version-gated, IIS-only, WP_Error-guarded self-heal migration that never fatals admin_init; and comprehensive DOM-level integration tests covering all edge cases in the approved spec (fresh file, foreign blocks, idempotency, partial failure, graceful skip). Real IIS-server validation is arranged separately by the user before final merge.

Mandatory Checklist

Code validation

I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
I triggered all changed lines of code at least once without new errors/warnings/notices.
I implemented built-in tests to cover the new/changed code.

Code style

I wrote a self-explanatory code about what it does.
I protected entry points against unexpected inputs.
I did not introduce unnecessary complexity.
Output messages (errors, notices, logs) are explicit enough for users to understand the issue and are actionnable.

Unticked items justification

N/A — all mandatory checklist items apply and are ticked.

Additional Checks

In the case of complex code, I wrote comments to explain it.
When possible, I prepared ways to observe the implemented system (logs, data, etc.)
I added error handling logic when using functions that could throw errors (HTTP/API request, filesystem, etc.)

Follow-up tickets

IIS: duplicate <preConditions> singleton collision in WebP/AVIF rewrite-rules classes #1180 — IIS: duplicate <preConditions> singleton collision in WebP/AVIF rewrite-rules classes (out of scope for this PR)

Pipeline status

DOD L2: ✅ PASS (1 non-blocking warning: Codacy diff-coverage check is ACTION_REQUIRED — https://app.codacy.com/gh/wp-media/imagify-plugin/pull-requests/1181/coverage)
Lead Review: ✅ PASS — Closes #509: Prevent duplicate staticContent blocks causing 500s on IIS #1181 (comment) area, full review comment on this PR
QA: ✅ PASS — Closes #509: Prevent duplicate staticContent blocks causing 500s on IIS #1181 (comment)

This PR is intentionally kept in draft pending the reporting user's own review and real IIS-server validation before merge (per the approved spec's locked decision — DOM-level integration tests are accepted as sufficient for the automated pipeline, but production IIS behavior has not been independently verified by a human).

@fileextension

IIS allows exactly one <staticContent> collection under system.webServer. Webp\IIS and Avif\IIS each emitted a whole wrapping <staticContent> element instead of merging into the shared one, so enabling both (or hitting a pre-existing foreign block) produced a second sibling and IIS failed to parse web.config, taking the whole site down with an HTTP 500. Root-cause fix: retarget both classes' get_raw_new_contents() to `/configuration/system.webServer/staticContent` and emit only a leaf <mimeMap>, letting the existing get_node()/prepend_node() recursion create or merge into the single container. AbstractIISDirConfFile now dedupes Imagify's own mimeMap entries by @fileextension (via a new get_owned_mime_extensions() hook) on both add() and remove() paths, since the leaf mimeMap no longer carries a `name` marker. Self-heal migration: add a version-gated (2.3.1) block in _imagify_new_upgrade() that collapses already-broken installs via a remove-then-add on both Webp\IIS and Avif\IIS, mirroring Display::activate() gating (display_nextgen only, no optimization_format branching). Guarded by a positive `if ( ! empty( $is_iis7 ) )` wrapper rather than an early return, so it never short-circuits any sibling migration block appended later, and every write is checked with is_wp_error() so a failed heal never fatals the upgrade routine. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

codacy-production · 2026-07-13T10:04:13Z

Not up to standards ⛔

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric Results

Duplication 0

View in Codacy

🔴 Coverage 14.81% diff coverage

Metric Results

Coverage variation Report missing for 3fb4102¹

Diff coverage ❌ 14.81% diff coverage (50.00%)

View coverage diff in Codacy

Coverage variation details

Coverable lines Covered lines Coverage

Common ancestor commit (3fb4102) Report Missing Report Missing Report Missing

Head commit (0c7aa75) 19556 639 3.27%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

Coverable lines Covered lines Diff coverage

Pull request (#1181) 27 4 14.81%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

¹ Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

Honemo · 2026-07-13T10:14:45Z

Note

Generated by the AI delivery pipeline (lead-reviewer · Claude Sonnet 5).

What changed: Root-cause fix for the IIS web.config duplicate <staticContent> bug (issue #509): Webp/IIS.php and Avif/IIS.php now emit only a leaf <mimeMap> targeted at the shared <staticContent> collection, and AbstractIISDirConfFile::insert_contents() dedupes by @fileExtension before the empty-contents early return. A version-gated self-heal migration in inc/admin/upgrader.php (2.3.1 block) collapses already-broken production installs via remove-then-add, gated by a positive if ( ! empty( $is_iis7 ) ) wrapper and unconditionally re-adding both WebP and AVIF (no optimization_format branching).

Review: ✅ PASS

All three challenger-flagged MUST_HAVEs verified correct in the actual diff (not just claimed):

fileExtension dedupe loop sits between the @name marker-removal loop and the ! $new_contents early return, so it runs on both add() and remove() paths.
Migration guard is if ( ! empty( $is_iis7 ) ) { ... } (positive wrapper), not an early return — future migration blocks appended after this one are unaffected.
Re-add is unconditional $webp->add() + $avif->add() gated only on display_nextgen, with no optimization_format XOR — matches the real Display::activate() gating.

Verified via full test run: composer test-unit -- --filter="IIS" (6/6) and composer test-integration -- --filter="AbstractIISDirConfFile" (14/14) both pass; composer phpcs and composer run-stan clean on all 4 changed source files. Manually traced the dedupe/merge logic through the fresh-file, foreign-block, both-format, and partial-failure scenarios — all converge to a single <staticContent> as required. <preConditions>/RewriteRules classes correctly untouched (out of scope, #1180).

Nice-to-haves:

inc/admin/upgrader.php — the migration's is_file_writable() pre-check duplicates the check add()/remove() already perform internally; harmless but could be simplified to rely solely on the internal WP_Error handling.
classes/WriteFile/AbstractIISDirConfFile.php — the fileExtension XPath is built via string concatenation ("...fileExtension='" . $extension . "'"); safe today since values are hardcoded constants (.webp/.avif), but worth a comment or a quoting helper if this ever becomes extensible.

Honemo · 2026-07-13T10:15:08Z

Note

Generated by the AI delivery pipeline (lead-reviewer · Claude Sonnet 5).

What changed: Root-cause fix for the IIS web.config duplicate <staticContent> bug (issue #509): Webp/IIS.php and Avif/IIS.php now emit only a leaf <mimeMap> targeted at the shared <staticContent> collection, and AbstractIISDirConfFile::insert_contents() dedupes by @fileExtension before the empty-contents early return. A version-gated self-heal migration in inc/admin/upgrader.php (2.3.1 block) collapses already-broken production installs via remove-then-add, gated by a positive if ( ! empty( $is_iis7 ) ) wrapper and unconditionally re-adding both WebP and AVIF (no optimization_format branching).

Review: ✅ PASS

All three challenger-flagged MUST_HAVEs verified correct in the actual diff (not just claimed):

fileExtension dedupe loop sits between the @name marker-removal loop and the ! $new_contents early return, so it runs on both add() and remove() paths.
Migration guard is if ( ! empty( $is_iis7 ) ) { ... } (positive wrapper), not an early return — future migration blocks appended after this one are unaffected.
Re-add is unconditional $webp->add() + $avif->add() gated only on display_nextgen, with no optimization_format XOR — matches the real Display::activate() gating.

Verified via full test run: composer test-unit -- --filter="IIS" (6/6) and composer test-integration -- --filter="AbstractIISDirConfFile" (14/14) both pass; composer phpcs and composer run-stan clean on all 4 changed source files. Manually traced the dedupe/merge logic through the fresh-file, foreign-block, both-format, and partial-failure scenarios — all converge to a single <staticContent> as required. <preConditions>/RewriteRules classes correctly untouched (out of scope, #1180).

Nice-to-haves:

inc/admin/upgrader.php — the migration's is_file_writable() pre-check duplicates the check add()/remove() already perform internally; harmless but could be simplified to rely solely on the internal WP_Error handling.
classes/WriteFile/AbstractIISDirConfFile.php — the fileExtension XPath is built via string concatenation ("...fileExtension='" . $extension . "'"); safe today since values are hardcoded constants (.webp/.avif), but worth a comment or a quoting helper if this ever becomes extensible.

Honemo · 2026-07-13T10:16:26Z

Note

Generated by the AI delivery pipeline (qa-engineer · claude-sonnet-5).

QA: PASS

Backend-only fix (IIS/Windows web.config XML generation) — no admin UI surface changed, local wp-env is not an IIS server, so validation was done via Strategy C (real PHPUnit suite executed locally, plus static/code analysis of the diff and the new test files to confirm they genuinely exercise the claimed scenarios with real DOMDocument assertions, not tautological ones).

Acceptance Criterion	Method	Result
1. Never more than one `<staticContent>` collection	Analysis + Test run	✅
2. Merges into existing (foreign or own) `<staticContent>` instead of duplicating	Analysis + Test run	✅
3. WebP + AVIF simultaneously in the same `<staticContent>`	Analysis + Test run	✅
4. Pre-existing broken sites self-heal via version-gated migration	Analysis + Test run	✅
5. Never fatals (malformed/unwritable/absent web.config, non-IIS, partial-failure)	Analysis + Test run	✅
6. `<preConditions>` singleton collision (#1180) is out of scope, untouched	`git diff` verification	✅

Evidence:

classes/WriteFile/AbstractIISDirConfFile.php::insert_contents() now dedupes owned <mimeMap> entries by @fileExtension (both add/remove paths) before delegating collection creation/merge to the existing get_node()/prepend_node(), so a second <staticContent> sibling can never be created.
classes/Webp/IIS.php and classes/Avif/IIS.php now emit only a leaf <mimeMap> fragment targeted at /configuration/system.webServer/staticContent (verified via Tests/Unit/classes/{Webp,Avif}/IIS/GetRawNewContentsTest.php — confirms no wrapping <staticContent> and no name= attribute is emitted).
inc/admin/upgrader.php adds a 2.3.1 version-gated block guarded by a positive if ( ! empty( $is_iis7 ) ) (never an early return), so it cannot block sibling migration blocks; remove-then-add sequence collapses duplicate Imagify-created siblings while preserving foreign blocks.
Ran the new integration suite directly: vendor/bin/phpunit --configuration Tests/Integration/phpunit.xml.dist --group WriteFile,IIS,Upgrader → 14/14 tests pass, 41 assertions (8 in InsertContentsTest, 6 in SelfHealMigrationTest), covering: fresh file, absent <staticContent>, foreign-block merge, webp+avif same-block, idempotent double-add, foreign-mimeMap-same-extension replace, remove-after-merge preserves foreign block, malformed web.config → WP_Error + file untouched, self-heal collapse (both formats, with foreign block preserved, display_nextgen off, non-IIS server untouched, conf-edition-disabled skip, partial-failure degraded-but-valid state).
Ran full unit suite: 266/266 tests pass, 554 assertions — matches the number claimed in the PR description exactly.
Ran full integration suite (excluding ajax/ms-files/external-http): 77/77 tests pass, 239 assertions, 2 pre-existing risky tests (GetNextgenCoverage, missing DB table in local env, unrelated to this change) — again matches the PR's claimed numbers exactly, confirming the reported results are real and not fabricated.
git diff origin/develop -- classes/Webp/RewriteRules/IIS.php classes/Avif/RewriteRules/IIS.php → empty diff, confirming the <preConditions> singleton issue (IIS: duplicate <preConditions> singleton collision in WebP/AVIF rewrite-rules classes #1180) was correctly left untouched.
Environment boot succeeded (bash bin/dev-start.sh exit 0, {E2E_URL} returns HTTP 200); plugin confirmed active on the PR branch. No UI/browser validation was applicable since this fix has no browser-visible surface (server-side XML generation for IIS only) and the local stack is not IIS.

No blockers found. No guards applicable — this is display-feature file-writer logic (web.config generation), independent of the Imagify API key/quota/license path, so no CANNOT_VERIFY was warranted.

Honemo added the Made by AI label Jul 13, 2026

Honemo self-assigned this Jul 13, 2026

Honemo mentioned this pull request Jul 13, 2026

Check if rules already exist in IIS (Windows) web.config file #509

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Closes #509: Prevent duplicate staticContent blocks causing 500s on IIS#1181

Closes #509: Prevent duplicate staticContent blocks causing 500s on IIS#1181
Honemo wants to merge 1 commit into
developfrom
fix/509-check-if-rules-already

Honemo commented Jul 13, 2026 •

edited

Loading

Uh oh!

codacy-production Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

Honemo commented Jul 13, 2026

Uh oh!

Honemo commented Jul 13, 2026

Uh oh!

Honemo commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Honemo commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Detailed scenario

What was tested

How to test

Affected Features & Quality Assurance Scope

Technical description

Documentation

New dependencies

Risks

Mandatory Checklist

Code validation

Code style

Unticked items justification

Additional Checks

Follow-up tickets

Pipeline status

Uh oh!

codacy-production Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Not up to standards ⛔

Uh oh!

Honemo commented Jul 13, 2026

Uh oh!

Honemo commented Jul 13, 2026

Uh oh!

Honemo commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Honemo commented Jul 13, 2026 •

edited

Loading

codacy-production Bot commented Jul 13, 2026 •

edited

Loading