HealthChecker: Fix IIS URL rewrite rule display bugs#2548
Open
dpaulson45 wants to merge 3 commits into
Open
Conversation
The detailed inbound and outbound URL rewrite rule display loops did not honor <remove> entries from inherited configuration sections. Rules that were explicitly removed via <remove name='...' /> in a parent web.config still appeared in the detailed rule table and could be incorrectly flagged as misconfigured. Collect exclude lists from each vDir's Remove.Name entries and skip matching rules in both inbound and outbound detailed display loops. Added mock data (AppHost Only Rule + remove directive) and test assertion to verify removed rules do not appear in detailed output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Some IIS locations like Microsoft-Server-ActiveSync/Proxy exist only in applicationHost.config and are not returned by Get-WebApplication. Get-URLRewriteRule previously only iterated WebConfigContent keys, so these appHost-only locations were skipped entirely and their inherited rewrite rules never appeared in the display. Build a combined location list from WebConfigContent keys plus any appHost location paths not already present. The existing walk-up logic handles null web.config content gracefully, so no loop body changes are needed. Added 3 unit tests using shared mock data covering appHost-only location processing, parent rule inheritance, and key deduplication. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a rewrite rule <match> element has extra attributes like negate or ignoreCase, Get-Member returns multiple properties causing \ to become an array. This broke the match value resolution, URL Match Problem check, and display — showing 'negate url - ' instead of 'url - .*'. Filter the property list to known match targets (url, serverVariable) before resolving the match value. Added mock rule with negate='true' and test assertion for correct MatchProperty display. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes display-layer issues in HealthChecker’s IIS URL rewrite rule reporting so results align with effective IIS configuration (inheritance + <remove> + <match> attribute parsing), and so appHost-only locations aren’t skipped.
Changes:
- Add support for processing IIS locations that exist only in
applicationHost.config(noGet-WebApplication/web.config entry). - Update inbound/outbound detailed display to exclude rules removed via
<remove name="..."/>. - Fix match-property resolution when
<match>includes additional attributes (e.g.,negate,ignoreCase) so display and “URL Match Problem” checks are correct.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Diagnostics/HealthChecker/Analyzer/Get-URLRewriteRule.ps1 | Builds a combined location list (web.config keys + appHost-only locations) so inheritance walk-up includes appHost-only vDirs. |
| Diagnostics/HealthChecker/Analyzer/Invoke-AnalyzerIISInformation.ps1 | Updates detailed inbound/outbound URL rewrite rule display logic for <remove> filtering and robust match-property selection. |
| Diagnostics/HealthChecker/Analyzer/Tests/Get-URLRewriteRule.Tests.ps1 | Adds unit tests covering appHost-only location processing/inheritance and key de-duplication. |
| Diagnostics/HealthChecker/Tests/HealthChecker.E19.Main.Tests.ps1 | Updates scenario assertions for <remove> filtering and match-property display correctness. |
| Diagnostics/HealthChecker/Tests/DataCollection/E19/Exchange/IIS/* | Extends mock IIS configs to include <remove> and <match negate="..."> scenarios for regression coverage. |
Comment on lines
+705
to
+710
| $excludeOutboundRules = @() | ||
| foreach ($section in $currentSection) { | ||
| if ($null -ne $section.Remove) { | ||
| $excludeOutboundRules += $section.Remove.Name | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
Valid edge case. The concern is about the "remove-then-readd" pattern where a rule is removed and re-added with different settings in the same section:
<rules>
<remove name="InheritedRule" />
<!-- Override inherited version with different config -->
<rule name="InheritedRule" stopProcessing="false">
<match url="different-pattern" />
<action type="Rewrite" url="new-destination" />
</rule>
</rules>IIS processes this as: remove the inherited version, then add the new local version. Our up-front exclude collection would incorrectly hide the re-added rule because the name matches the <remove> entry.
However, this pattern is not used in Exchange environments. The typical Exchange pattern is a simple <remove> to exclude an inherited rule, not remove-then-readd to override it. Adding ordered section processing would increase complexity without practical benefit for HealthChecker's target scenarios. If this pattern surfaces in the wild, we can revisit.
Comment on lines
+615
to
+619
| # Collect <remove> entries so inherited rules that are removed at a lower level are excluded. | ||
| $excludeRules = @() | ||
| foreach ($section in $currentSection) { | ||
| if ($null -ne $section.Remove) { | ||
| $excludeRules += $section.Remove.Name |
Member
Author
There was a problem hiding this comment.
Same response as the inbound comment — this is the same pattern applied to outbound rules. Not a concern for Exchange environments.
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
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.
Summary
Fixes three display-layer bugs in the HealthChecker IIS URL rewrite rule processing pipeline. These were discovered during server testing as follow-up to #2545 (outbound rewrite rule support).
Changes
1. Detailed display ignores
<remove>directives (Fixes #2003)The detailed inbound and outbound URL rewrite rule display loops did not honor
<remove>entries from inherited configuration sections. Rules explicitly removed via<remove name='...' />in a parent web.config still appeared in the detailed rule table and could be incorrectly flagged as misconfigured.Fix: Collect exclude lists from each vDir's
Remove.Nameentries and skip matching rules in both inbound and outbound detailed display loops.2. AppHost-only IIS locations missing from rewrite rule processing
Some IIS locations like
Microsoft-Server-ActiveSync/Proxyexist only inapplicationHost.configand are not returned byGet-WebApplication.Get-URLRewriteRulepreviously only iteratedWebConfigContentkeys, so these locations were skipped entirely — their inherited rewrite rules never appeared in the summary or detailed display.Fix: Build a combined location list from
WebConfigContentkeys plus any appHost location paths not already present. The existing walk-up logic handles null web.config content gracefully.3. Match property array bug when match has extra attributes (Fixes #2024)
When a rewrite rule
<match>element has extra attributes likenegateorignoreCase,Get-Memberreturns multiple properties causing\to become an array. This broke the match value resolution, URL Match Problem check, and display — showingnegate url -instead ofurl - .*.Fix: Filter the property list to known match targets (
url,serverVariable) before resolving the match value.Testing
Get-URLRewriteRule(3 new for appHost-only locations)Main.Tests.ps1(updated assertions for remove filtering and match property)Closes #2003
Closes #2024