Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect binding redirects generated for SqlClient's runtime dependencies #2541

Open
benjamin-hodgson opened this issue May 29, 2024 · 12 comments

Comments

@benjamin-hodgson
Copy link

benjamin-hodgson commented May 29, 2024

Describe the bug

SqlClient was compiled with Azure.Core version 1.35.0.0. When building a project using version 1.38.0.0 (or indeed any other version), the appropriate binding redirect is not generated. This results in FileLoadExceptions in the AD authentication codepath - example:

Unhandled Exception: System.IO.FileLoadException: Could not load file or assembly 'Azure.Core, Version=1.35.0.0, Culture=neutral, PublicKeyToken=92742159e12e44c8' or one of its dependencies. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)
   at Microsoft.Data.SqlClient.ActiveDirectoryAuthenticationProvider.AcquireTokenAsync(SqlAuthenticationParameters parameters)
   at Program.Main() in D:\test\Program.cs:line 7

(My minimal repro, below, trips on Azure.Core but I've also seen errors mentioning ValueTuple and System.Threading.Tasks.Extensions in other contexts. I believe they're all due to the same issue.)

The problem here is that the ResolveAssemblyReferences target (which is responsible for determining which libraries need binding redirects) consumes reference assemblies from Nuget (when they exist), and SqlClient's reference assemblies do not declare the library's runtime dependency on Azure.Core.

The reference assembly in ILSpy (ref\net462\Microsoft.Data.SqlClient.dll):
ref\net462\Microsoft.Data.SqlClient.dll

The code assembly (lib\net462\Microsoft.Data.SqlClient.dll):
lib\net462\Microsoft.Data.SqlClient.dll

This means ResolveAssemblyReferences doesn't realise there's a conflict (it thinks SqlClient doesn't require Azure.Core at all) so fails to generate the correct binding redirects. When SqlClient's code assembly is deployed into the bin folder and run, it requires version 1.35, and, absent the required binding redirect, it dies because it can only find 1.38.

To reproduce

The issue manifests when the other assemblies in your compilation all reference v1.38 of the Azure.Core assembly. This combination of versions reproduces the issue (though there are other scenarios that produce the same issue with various other assemblies):

<!-- test.csproj -->
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net472</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Azure.Identity" Version="1.11.2" />
    <PackageReference Include="Microsoft.Data.SqlClient" Version="5.2.0" />
  </ItemGroup>

</Project>
// Program.cs
using Microsoft.Data.SqlClient;

public class Program
{
    public static void Main()
    {
        // We just need to cause AcquireTokenAsync to be jitted.
        // In practice this'll typically happen during a call to
        // SqlConnection.Open() when authenticating via AD
        new ActiveDirectoryAuthenticationProvider().AcquireTokenAsync(null);
    }
}

Build this project and observe the lack of binding redirect for Azure.Core (and others, eg Tasks.Extensions) in bin\Debug\net472\test.exe.config:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <startup>
    <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.7.2" />
  </startup>
  <runtime>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="Microsoft.Identity.Client" publicKeyToken="0a613f4dd989e8ae" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.60.3.0" newVersion="4.60.3.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Buffers" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.0.3.0" newVersion="4.0.3.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Diagnostics.DiagnosticSource" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-6.0.0.1" newVersion="6.0.0.1" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Runtime.CompilerServices.Unsafe" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-6.0.0.0" newVersion="6.0.0.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Security.AccessControl" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-6.0.0.0" newVersion="6.0.0.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Security.Cryptography.ProtectedData" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.0.5.0" newVersion="4.0.5.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Text.Encodings.Web" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-6.0.0.0" newVersion="6.0.0.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Text.Json" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.0.1.2" newVersion="4.0.1.2" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Threading.Tasks.Extensions" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.2.0.1" newVersion="4.2.0.1" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.ValueTuple" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.0.3.0" newVersion="4.0.3.0" />
      </dependentAssembly>
    </assemblyBinding>
  </runtime>
</configuration>

Running the project using dotnet run causes the exception above.

Expected behavior

The reference assemblies for SqlClient should correctly declare all of the library's (immediate) runtime dependencies. This will allow ResolveAssemblyReference to solve the versions correctly and generate the required binding redirects.

Further technical details

Microsoft.Data.SqlClient version: 5.2.0 or 3.1.5 (but probably others too)
.NET target: net472
SQL Server version: Any
Operating system: Windows 11

Additional context
Probably not relevant, but: I found this issue in a large repo with thousands of projects. The repo is in the process of being migrated off of CoreXT and onto CentralPackageVersions.

@benjamin-hodgson
Copy link
Author

benjamin-hodgson commented May 29, 2024

I'm guessing #2414 would probably fix the issue once and for all if/when it gets merged.

@benjamin-hodgson benjamin-hodgson changed the title SqlClient's reference assemblies should declare all dependencies Incorrect binding redirects generated for SqlClient's runtime dependencies May 29, 2024
@benjamin-hodgson
Copy link
Author

benjamin-hodgson commented May 29, 2024

Is this a dupe of #1900 ? If so I think that issue probably shouldn't have been closed.

@arellegue arellegue added the 🆕 Triage Needed For new issues, not triaged yet. label May 29, 2024
@arellegue
Copy link
Contributor

We will investigate this issue and we'll get back to you when we have an update.

@kf-gonzalez2
Copy link

@benjamin-hodgson Hotfix 5.2.1 is out now, would you mind testing with this version and letting us know if this is still an issue please?

@kf-gonzalez2 kf-gonzalez2 added ℹ️ Needs more Info Issues that have insufficient information to pursue investigations and removed 🆕 Triage Needed For new issues, not triaged yet. labels Jun 4, 2024
@benjamin-hodgson
Copy link
Author

benjamin-hodgson commented Jun 4, 2024

Looks like this'll work for now, at least for our purposes, but it's liable to become broken again as new versions of the upstream libraries come out. The root cause is the missing references in the ref assemblies. (I don't think this issue should be closed.)

Thanks for turning this hotfix around so quickly!

@dauinsight dauinsight added 🆕 Triage Needed For new issues, not triaged yet. and removed ℹ️ Needs more Info Issues that have insufficient information to pursue investigations labels Jun 5, 2024
@kf-gonzalez2 kf-gonzalez2 removed the 🆕 Triage Needed For new issues, not triaged yet. label Jun 11, 2024
@zoeperryman
Copy link

With the release of Azure.Identity 1.12 (and Azure.Core 1.40) this is broken again. My full framework projects (net462 and net48) fail to automatically generate the binding redirects, it needs to be done manually.

Referencing the following project combination:

    <PackageReference Include="Azure.Identity" Version="1.12.0" />
    <PackageReference Include="Microsoft.Data.SqlClient" Version="5.2.2" />

Results in the following error:

Microsoft.Data.SqlClient.SqlException : Could not load file or assembly 'Azure.Core, Version=1.38.0.0, Culture=neutral, PublicKeyToken=92742159e12e44c8' or one of its dependencies.

Adding the following bindings manually to a app/web.config resolves the problem. But this should be done automatically when automatic binding redirects are enabled.

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <runtime>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="Azure.Core" publicKeyToken="92742159e12e44c8" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.40.0.0" newVersion="1.40.0.0" />
      </dependentAssembly>
      <dependentAssembly>
        <assemblyIdentity name="Microsoft.Identity.Client" publicKeyToken="0a613f4dd989e8ae" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.61.3.0" newVersion="4.61.3.0" />
      </dependentAssembly>
      <dependentAssembly>
        <assemblyIdentity name="Azure.Identity" publicKeyToken="92742159e12e44c8" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.12.0.0" newVersion="1.12.0.0" />
      </dependentAssembly>
    </assemblyBinding>
  </runtime>
</configuration>

@JRahnama
Copy link
Contributor

@zoeperryman Do you suggest that this should be handled on the driver side or as a general workaround? I believe it would be overstepping for the driver to make decisions regarding user-defined redirects or bindings.

@zoeperryman
Copy link

I'm suggesting that the binding redirects should automatically be generated when referencing later versions of Azure (and other) libraries then what Microsoft.Data.SqlClient is compiled against. This works for all the other nuget packages out there (well most I guess), what is different or broken about this one?

@benjamin-hodgson's original post seemed to suggest this was a problem with the reference assemblies for Microsoft.Data.SqlClient in the nuget file.

The fact that these bindings don't get added automatically means the bindings need to be manually added and maintained which is a pain and hugely error prone when you have dozens of solutions that require it.

@benjamin-hodgson
Copy link
Author

@JRahnama This is a real bug in the way the SqlClient package is distributed - please see my original detailed bug report.

It was worked-around in v5.2.1 by rebuilding SqlClient against the latest version of Azure.Identity/Azure.Core but as predicted it became broken again when new versions of those upstream packages were released.

To fix the bug you could either update your dependencies once again (ie, continue working around it) or get #2414 merged

@edwardneal
Copy link
Contributor

@benjamin-hodgson I think this should be fixed (in the absence of auto-generated reference assemblies) by #2878.

I've synced the references between the runtime assembly and the reference assembly, and pushed it through CI. Could you test with the package from this CI build please? The direct download URL of the ZIP file containing the .nupkg is here

To test, I added a reference to the version of the package (6.0.0-pull.126701, which has a transitive reference to Azure.Identity 1.11.4), then to Azure.Identity 1.12.0. I can see that VS' automatic binding redirects have added a few new entries to app.config. The relevant excerpt:

<dependentAssembly>
  <assemblyIdentity name="Azure.Core" publicKeyToken="92742159e12e44c8" culture="neutral" />
  <bindingRedirect oldVersion="0.0.0.0-1.40.0.0" newVersion="1.40.0.0" />
</dependentAssembly>
<dependentAssembly>
  <assemblyIdentity name="Azure.Identity" publicKeyToken="92742159e12e44c8" culture="neutral" />
  <bindingRedirect oldVersion="0.0.0.0-1.12.0.0" newVersion="1.12.0.0" />
</dependentAssembly>

@JRahnama
Copy link
Contributor

Here is the package from that build. Simply change the extension to .nupkg. Please note, this is an unsigned test package and is not intended for production use.

Microsoft.Data.SqlClient.6.0.0-pull.126701.zip

@edwardneal
Copy link
Contributor

Thanks @JRahnama - and yes, it's definitely not intended for production use. This package isn't designed to be anything more than a sanity check to make sure that the downstream projects referencing SqlClient build and that VS generates the correct binding redirects in the output app.config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs Investigation
Development

No branches or pull requests

7 participants