[vs17.14] Remove the ConfigurationManager binding redirect - #14560
[vs17.14] Remove the ConfigurationManager binding redirect#14560AlesProkop wants to merge 1 commit into
Conversation
…g redirect Fixes dotnet#14404 MSBuild.exe.config (x86) and MSBuild.exe.config (amd64/arm64) redirect System.Configuration.ConfigurationManager to 10.0.0.8, and the amd64 config adds a codeBase pointing at MSBuild\Current\Bin. That assembly is not part of Microsoft.Build.vsix, so any in-proc task carrying its own ConfigurationManager gets its bind hijacked to a file that does not exist and fails to load. No MSBuild assembly needs the redirect. On .NET Framework the System.Configuration.ConfigurationManager reference assembly only type-forwards into the GAC's System.Configuration, so the compiled binaries reference System.Configuration 4.0.0.0 and never bind to the package assembly. Remove both dependentAssembly entries so task-local ConfigurationManager binds resolve the way they did before 17.14.51. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98ea148f-b805-4636-8f5a-48016ad923d5
08d7bd9 to
975508d
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes the System.Configuration.ConfigurationManager binding redirect (and associated codeBase) from MSBuild’s .NET Framework app configs to prevent host-level binding policy from hijacking task-local ConfigurationManager loads—addressing a VSIX packaging regression reported in #14404. It also bumps the servicing version for the vs17.14 branch.
Changes:
- Remove the
System.Configuration.ConfigurationManagerdependentAssemblybinding redirect fromsrc/MSBuild/app.config. - Remove the same redirect +
codeBasefromsrc/MSBuild/app.amd64.config, replacing it with an explanatory comment referencing #14404. - Bump
VersionPrefixto17.14.54.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/MSBuild/app.config | Removes ConfigurationManager binding redirect; adds a comment explaining why MSBuild shouldn’t redirect it on .NET Framework. |
| src/MSBuild/app.amd64.config | Removes ConfigurationManager binding redirect and codeBase that could point to a missing VSIX payload file; adds rationale comment. |
| eng/Versions.props | Increments the servicing VersionPrefix to 17.14.54. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Expert Code Review — PR #14560
✅ 24/24 dimensions clean — no findings.
This is a targeted servicing fix that removes a broken binding redirect for System.Configuration.ConfigurationManager from the .NET Framework MSBuild config files.
Analysis
Technical correctness: The explanation in the PR description is accurate. On .NET Framework, the NuGet package System.Configuration.ConfigurationManager (publicKeyToken cc7b13ffcd2ddd51) is a type-forwarding shim that delegates to the GAC's System.Configuration 4.0.0.0. MSBuild assemblies compiled against this reference end up binding to System.Configuration 4.0.0.0 at runtime, never to the NuGet package assembly. The redirect to version 10.0.0.8 with a codeBase pointing to a file not present in Microsoft.Build.vsix was actively harmful: any in-proc task carrying its own copy of System.Configuration.ConfigurationManager had its binding hijacked to a non-existent path, causing a load failure.
Backwards compatibility: Removing the redirect restores the pre-17.14.51 behavior. This is a regression fix, not a new behavioral change, so no ChangeWave gate is required.
VS Servicing model: The version bump 17.14.53 → 17.14.54 in eng/Versions.props is appropriate for a patch-level servicing release on the vs17.14 branch.
Asymmetry between configs: app.config (x86) had the redirect without a codeBase element, while app.amd64.config had it with a codeBase. Both are removed correctly — the codeBase is strictly more harmful (explicit missing-file reference), but the redirect itself was also unnecessary and was silently shadowing any task-local version.
Test coverage: No regression test is included. Binding redirect behavior in .NET Framework requires a full runtime host to exercise and is not easily unit-testable. For a servicing fix of this scope this is acceptable, though an integration test or manual repro in the linked issue (#14404) would be valuable documentation of the scenario.
Security, concurrency, performance, API surface: All N/A for this change — it is purely a config file edit.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
awmgmcpg
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
Generated by Expert Code Review (on open) for #14560 · sonnet46 · 51.9 AIC · ⌖ 5.17 AIC · ⊞ 4.9K
Fixes #14404
Context
MSBuild.exe.configredirectsSystem.Configuration.ConfigurationManagerto10.0.0.8, and the amd64/arm64 config adds acodeBasepointing atMSBuild\Current\Bin\System.Configuration.ConfigurationManager.dll. That assembly is not inMicrosoft.Build.vsix, so an in-proc task that carries its own ConfigurationManager has its bind hijacked to a file that does not exist and fails withFileNotFoundException. Regression from #14127 (17.14.40 -> 17.14.51).The redirect is not needed by MSBuild. On .NET Framework the
System.Configuration.ConfigurationManagerreference assembly only type-forwards into the GAC'sSystem.Configuration, so the shipped binaries referenceSystem.Configuration, Version=4.0.0.0and never bind to the package assembly:This is the alternative to #14496, which instead added the missing assembly to the VSIX payload.
Changes Made
System.Configuration.ConfigurationManagerdependentAssemblyentry fromsrc/MSBuild/app.configandsrc/MSBuild/app.amd64.config(redirect +codeBase), leaving a comment explaining why there is none.Task-local
System.Configuration.ConfigurationManagerbinds now resolve the way they did before 17.14.51.Testing
./build.cmd -v quiet /p:RunAnalyzers=false—ValidateMSBuildPackageDependencyVersionspasses for x86, x64 and arm64. The only failure is the pre-existing local-only arcade OptProfGenerateTrainingInputFileserror, which reproduces unchanged on the unmodified base commit.MSBuild.exe.configfor all three platforms no longer contains a ConfigurationManagerdependentAssemblyand that the generatedNuGetFrameworkWrapper.redirects.cs(which mirrors these redirects into the task AppDomain config) no longer contains one either.