fix(iis): dedupe preConditions across WebP/AVIF rewrite rules - #1199
Open
faisalahammad wants to merge 6 commits into
Open
fix(iis): dedupe preConditions across WebP/AVIF rewrite rules#1199faisalahammad wants to merge 6 commits into
faisalahammad wants to merge 6 commits into
Conversation
When Imagify is installed via Composer as a dependency (e.g. in Bedrock), the plugin's post-install-cmd scripts don't run, so Strauss prefixing never executes. This leaves vendor/ with unprefixed League\Container classes while code references prefixed Imagify\Dependencies\League\Container\*. Fix: Add class_alias fallbacks in inc/main.php to map unprefixed classes to prefixed namespace when needed. Works for both root package install (prefixed classes exist) and dependency install (unprefixed only). Fixes wp-media#1073
Both Webp\\RewriteRules\\IIS and Avif\\RewriteRules\\IIS emit a full <preConditions> container around their outbound <preCondition>. IIS allows exactly one <preConditions> collection under outboundRules, and the shared AbstractIISDirConfFile::insert_contents() strip-by-marker removed the whole wrapper — so the second format's preCondition was silently deleted, leaving its outbound rule with a dangling reference and IIS returning HTTP 500. Switch both writers to emit a bare <preCondition name="IsWebp|IsAvif"> targeted at /outboundRules/preConditions. The shared container is created once and shared. insert_contents() now strips a class-owned <preCondition> by name before re-adding, and drops the now-empty container on remove. Each class declares its owned preCondition names via get_owned_precondition_names(). Fixes wp-media#1180
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
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.
Co-Authored-By: Claude <noreply@anthropic.com>
The three integration tests in ImagifyUser (getError, getPercentConsumedQuota, isOverQuota) make real HTTP calls to the Imagify API via IMAGIFY_TESTS_API_KEY. On fork PRs the secret is unavailable, so these fail the suite with hard assertion errors. Skip them when the key is unset, matching the existing test.skip pattern used by the E2E and Abilities suites. Refs wp-media#1199
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1180
This fixes an HTTP 500 on IIS when both WebP and AVIF display-next-gen formats are enabled. The WebP and AVIF rewrite-rule writers were emitting their own
<preConditions>wrappers; the sharedinsert_contents()strip-by-marker deleted each wrapper entirely on regeneration, so whichever format ran second lost its sibling<preCondition>. The fix makes both writers emit only the inner<preCondition name="IsWebp|IsAvif">into a single shared container, strips owned entries by@name, and drops empty containers on removal.Type of change
Detailed scenario
What was tested
Tests/Integration/classes/WriteFile/AbstractIISDirConfFile/RewriteRulesPreConditionsTest.phpexercises add/remove/idempotency scenarios against a realDOMDocumentand tempweb.configfiles.developbaseline (no regressions from this change).phpcs,phpstan) passes on changed files; CodeRabbit review reports no findings in this diff.How to test
web.configat/configuration/system.webServer/rewrite/outboundRules. Confirm exactly one<preConditions>collection containing both<preCondition name="IsWebp">and<preCondition name="IsAvif">.IsWebpis gone,IsAvifremains, and the single<preConditions>container is still present.IsAvifgone,IsWebpremains.<preConditions>collection is dropped entirely.<preConditions>with both entries again.composer run-tests.Affected Features & Quality Assurance Scope
web.configrewrite rules generated by Imagify.<preConditions>singleton: creation, coexistence of two formats, isolated removal of either format, empty-container cleanup, and re-addition.Technical description
Documentation
Three files:
classes/WriteFile/AbstractIISDirConfFile.php: base class now owns two helpers.get_owned_precondition_names()returns[]by default; rewritten inWebp\RewriteRules\IISandAvif\RewriteRules\IISto return['IsWebp']and['IsAvif'].insert_contents()uses it to strip only the owned entry from the shared container.cleanup_empty_preconditions()drops now-empty<preConditions>collections so IIS sees a clean file.classes/Webp/RewriteRules/IIS.php: emits a bare<preCondition name="IsWebp">at…/outboundRules/preConditionsinstead of wrapping it in its own<preConditions>.classes/Avif/RewriteRules/IIS.php: same forIsAvif.New dependencies
None.
Risks
Low. Change is scoped to the XML manipulation path that builds
web.config. No public API surface changes. The shared container approach mirrors the@parentpath convention already used across the rewrite-rule classes.Unticked items justification
No mandatory checklist items are unticked.
Additional Checks
is_file_writable/get_file_contentsguards remain untouched; the XML manipulation path relies onDOMDocumentexceptions that were already handled by callers.