search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.27k stars 4.73k forks source link

Where should I reach out about publishing a new patch for System.Text.Encoding? #86671

Closed Bartleby2718 closed 1 year ago

Bartleby2718 commented 1 year ago

System.Text.Encoding 4.3.0 (i.e. the latest version) depends on System.Runtime 4.3.0, which in turn depends on runtime.any.System.Runtime 4.3.0 (if you specify a RuntimeIdentifier like linux-x64), which in turn depends on a vulnerable package System.Private.Uri 4.3.0.

I know dotnet/runtime is probably not the right place to ask, but I thought you'd know whom I should reach out to. Who will be able to publish a new patch for System.Text.Encoding and System.Runtime? Only after then can I reach out to the owners of hundreds of NuGet packages that use those.

Thanks in advance.

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-text-encoding See info in area-owners.md if you want to be subscribed.

Issue Details
`System.Text.Encoding` 4.3.0 (i.e. [the latest version](https://www.nuget.org/packages/System.Text.Encoding/#versions-body-tab)) depends on `System.Runtime` 4.3.0, which in turn depends on `runtime.any.System.Runtime` 4.3.0 (_if_ you specify a `RuntimeIdentifier` like `linux-x64`), which in turn depends on a vulnerable package `System.Private.Uri` 4.3.0. - I don't know how the vulnerability can be exploited, but I don't think `System.Text.Encoding` (or `System.Runtime`) should be depending on a vulnerable package. - I can of course add a new direct `PackageReference` for `System.Private.Uri` 4.3.2, but https://www.nuget.org/packages/System.Private.Uri/#readme-body-tab says: > Internal implementation package not meant for direct consumption. Please do not reference directly. I know dotnet/runtime is probably not the right place to ask, but I thought you'd know whom I should reach out to. Who will be able to publish a new patch for `System.Text.Encoding` and `System.Runtime`? Only after then can I reach out to the owners of [hundreds of NuGet packages that use those](https://www.nuget.org/packages/System.Text.Encoding/#usedby-body-tab). Thanks in advance.
Author: Bartleby2718
Assignees: -
Labels: `area-System.Text.Encoding`, `untriaged`
Milestone: -
ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries See info in area-owners.md if you want to be subscribed.

Issue Details
`System.Text.Encoding` 4.3.0 (i.e. [the latest version](https://www.nuget.org/packages/System.Text.Encoding/#versions-body-tab)) depends on `System.Runtime` 4.3.0, which in turn depends on `runtime.any.System.Runtime` 4.3.0 (_if_ you specify a `RuntimeIdentifier` like `linux-x64`), which in turn depends on a vulnerable package `System.Private.Uri` 4.3.0. - I don't know how the vulnerability can be exploited, but I don't think `System.Text.Encoding` (or `System.Runtime`) should be depending on a vulnerable package. - I can of course add a new direct `PackageReference` for `System.Private.Uri` 4.3.2, but https://www.nuget.org/packages/System.Private.Uri/#readme-body-tab says: > Internal implementation package not meant for direct consumption. Please do not reference directly. I know dotnet/runtime is probably not the right place to ask, but I thought you'd know whom I should reach out to. Who will be able to publish a new patch for `System.Text.Encoding` and `System.Runtime`? Only after then can I reach out to the owners of [hundreds of NuGet packages that use those](https://www.nuget.org/packages/System.Text.Encoding/#usedby-body-tab). Thanks in advance.
Author: Bartleby2718
Assignees: -
Labels: `area-Infrastructure-libraries`, `untriaged`
Milestone: -
teo-tsirpanis commented 1 year ago

Hello @Bartleby2718 are you using this package yourself or as a transitive dependency of another package? In the former case you can just remove it, and in the latter case you should contact the package's maintainers to remove it.

In general, if a System.* package is stuck on version 4.3.x and has not received an update in years, you should just remove it; chances are that your app will keep wworking without any problem.

Bartleby2718 commented 1 year ago

@teo-tsirpanis Thanks for the response. It's the latter; I get it transitively in various ways.

For example, Microsoft.Data.SqlClient 5.1.1 depends on Microsoft.IdentityModel.JsonWebTokens 6.24.0, which in turn depends on System.Text.Encoding 4.3.0. In this case, should I first contact https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet (maintainers of Microsoft.IdentityModel.JsonWebTokens), have them publish a new patch, and then ask https://github.com/dotnet/SqlClient (maintainers of Microsoft.Data.SqlClient) to do the same?

huoyaoyuan commented 1 year ago

See also #85641.

If your entry application is targeting .NET 5+, then every such System.* package of version 4.x will be effectively no-op.

ViktorHofer commented 1 year ago

If your entry application is targeting .NET 5+, then every such System.* package of version 4.x will be effectively no-op.

Those packages are still being restored and potentially marked as vulnerable. Ideally we would avoid these unnecessary dependencies in the entire stack.

For example, Microsoft.Data.SqlClient 5.1.1 depends on Microsoft.IdentityModel.JsonWebTokens 6.24.0, which in turn depends on System.Text.Encoding 4.3.0. In this case, should I first contact https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet (maintainers of Microsoft.IdentityModel.JsonWebTokens), have them publish a new patch, and then ask https://github.com/dotnet/SqlClient (maintainers of Microsoft.Data.SqlClient) to do the same?

@Bartleby2718 yes, we should start with https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet and file an issue and potentially a follow-up PR to avoid the dependency. The API exposed by System.Text.Encoding is available inbox without an extra package dependency in all current officially supported target frameworks.

Note that there are more examples i.e. System.Net.Http which shouldn't be referenced anymore via a package reference as the API is inbox as well on all current officially supported TFMs. Another example is System.Reflection.Metadata which is inbox on .NETCoreApp TFMs.

Bartleby2718 commented 1 year ago

@ViktorHofer That makes sense to me. A quick rookie question: could you give me an example of how to replace an API exposed by System.Text.Encoding? Curious what the recommended alternative is.

ViktorHofer commented 1 year ago

You can continue to use the APIs exposed by System.Text.Encoding. You just don't want to consume them via its package as the minimum officially supported modern TFMs net6.0, net462 and netstandard2.0 all support that functionality in-the-box.

As an example, the System.Text.Encoding assembly includes the following public API: System.Text.EncoderFallback. https://apisof.net is a great website to search for API and see which frameworks include them.

When looking at https://apisof.net/catalog/2cbe18da-fa5b-5b14-efee-7ecbe589c786?fx=netstandard2.0 you will see that the API is available in-the-box in all TFMs (the green color indicates that). Note that when targeting netstandard1.x the legacy System.Text.Encoding package actually is referenced (transitively) but I wouldn't recommend still targeting netstandard1x these days as the API is super limited and the dependency graph is huge.

Bartleby2718 commented 1 year ago

TIL about the website! However, I'm not sure why that method doesn't work in my case. Do you have any idea why it fails in the case below, @ViktorHofer?

We have an internal NuGet package whose latest version is 1.4.0. I see the following in Our.Internal.App's packages.lock.json file:

      "Our.Internal.NuGet.Package": {
        "type": "Transitive",
        "resolved": "1.4.0",
        ...
        "dependencies": {
          ...
          "System.Linq": "4.3.0"
        }
      },

However, if I take a look at Our.Internal.NuGet.Package's csproj file or nuspec file, I don't see System.Linq. However, I do see using System.Linq; in some files in the project. If I remove it, I get a compilation error because the extension method Enumerable.FirstOrDefault() in the System.Linq namespace)) cannot be found. However, I see all green in https://apisof.net/catalog/69368780-af74-d211-d611-6823d614cdba, so I'm very confused.

Potentially relevant pieces of info:

teo-tsirpanis commented 1 year ago

My guess is that one of Our.Internal.NuGet.Package's direct or transitive dependencies targets .NET Standard 1.x. If you post your app's .deps.json file (after redacting sensitive information), it would be very helpful.

Bartleby2718 commented 1 year ago

@teo-tsirpanis Okay I think I've found a few relevant pieces of information from Our.Internal.NuGet.Package\bin\Debug\netcoreapp3.1\Our.Internal.NuGet.Package.deps.json, but they make me even more confused:

{
  "runtimeTarget": {
    "name": ".NETCoreApp,Version=v3.1",
    "signature": ""
  },
  "compilationOptions": {},
  "targets": {
    ".NETCoreApp,Version=v3.1": {
      "Our.Internal.NuGet.Package/1.0.0": {
        "dependencies": {
          // next line is weird because Our.Internal.NuGet.Package.nuspec requires that Second.Internal.NuGet.Package be [2.12.0, 3.0.0) for both all of its TargetFrameworks
          "Second.Internal.NuGet.Package": "1.0.0", 
          ... // other seemingly irrelevant packages
          "Autofac": "4.9.4",
          "EntityFramework": "6.3.0",
          "Microsoft.SourceLink.Bitbucket.Git": "1.0.0",
          "Newtonsoft.Json": "12.0.3",
          "StyleCop.Analyzers": "1.2.0-beta.406"
        },

According to the Our.Internal.NuGet.Package.deps.json, the chain is:

"Second.Internal.NuGet.Package/1.0.0"
-> "Microsoft.Data.SqlClient/2.0.0"
-> "Microsoft.Identity.Client/4.14.0"
-> "System.ComponentModel.TypeConverter/4.3.0"
-> "System.Linq": "4.3.0"

I think MSBuild is incorrectly defaulting the version of Second.Internal.NuGet.Package to 1.0.0 because Our.Internal.NuGet.Package has a ProjectReference on Second.Internal.NuGet.Package. Is this a bug? (Seems like it should've been fixed in https://github.com/NuGet/Home/issues/3874.)

teo-tsirpanis commented 1 year ago

Can you update to the latest version of Microsoft.Data.SqlClient?

Bartleby2718 commented 1 year ago

Ah, I think that's the culprit. I think we forgot to update it in internal libraries. 😅

ViktorHofer commented 1 year ago

Just submitted a PR in azure-activedirectory-identitymodel-extensions-for-dotnet to avoid the package references pointing to old / deprecated packages: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/2098. That includes the removal of the System.Text.Encoding/4.3.0 dependency.