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

Could not load file or assembly 'System.Security.Permissions..' when building projects with dotnet #4808

Closed
costin-zaharia-sonarsource opened this issue Aug 19, 2021 · 18 comments · Fixed by #5049
Assignees
Labels
Area: C# C# rules related issues. Type: Bug Exceptions and blocking issues during analysis.
Milestone

Comments

@costin-zaharia-sonarsource
Copy link
Member

Source: https://community.sonarsource.com/t/azure-devops-pipeline-build-fails-with-sonarcloud-and-source-generator/45725

Context:

  • sonar-dotnet embeds the Newtonsoft.Json.dll library compiled for net45.
  • Our user is using a code generator project which targets netstandard2.0 and is compiled for the net5 runtime.

Stack Trace:

Message: Could not load file or assembly 'System.Security.Permissions, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. The system cannot find the file specified.
   at Newtonsoft.Json.Serialization.JsonTypeReflector.get_DynamicCodeGeneration()
   at Newtonsoft.Json.Serialization.JsonTypeReflector.get_ReflectionDelegateFactory()
   at Newtonsoft.Json.Serialization.DefaultContractResolver.GetDefaultCreator(Type createdType)
   at Newtonsoft.Json.Serialization.DefaultContractResolver.InitializeContract(JsonContract contract)
   at Newtonsoft.Json.Serialization.DefaultContractResolver.CreateArrayContract(Type objectType)
   at Newtonsoft.Json.Serialization.DefaultContractResolver.CreateContract(Type objectType)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Newtonsoft.Json.Utilities.ThreadSafeStore`2.Get(TKey key)
   at Newtonsoft.Json.Serialization.DefaultContractResolver.ResolveContract(Type type)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.GetContract(Type type)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.GetContractSafe(Type type)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonSerializer.Deserialize(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value)
   at SourceGen.Generator.Execute(GeneratorExecutionContext context)

The problem appears when JsonTypeReflector.get_DynamicCodeGeneration is called since .Net Core tries to resolve SecurityPermission from System.Security.Permissions.dll which cannot be found. Source code: https://github.com/JamesNK/Newtonsoft.Json/blob/11.0.2/Src/Newtonsoft.Json/Serialization/JsonTypeReflector.cs#L449

The type was made obsolete starting with .Net 5: https://docs.microsoft.com/en-us/dotnet/api/system.security.permissions.securitypermission?view=net-5.0

There seem to be differences when building with MSBuild vs dotnet since MSBuild will not even try to load the System.Security.Permissions.dll assembly.

I’ve done a short test and, if I embed the netstandard2.0 version of Newtonsoft.Json.dll, I can build the repro project without any problems. The limitation with this approach is that we will have compatibility issues with older versions of Visual Studio.

As far as I can tell the compiler will always load the version of Newtonsoft.Json we provide. Seems to be related to this issue: dotnet/roslyn#41421.

One possible solution is to remove the dependency on Json.Net.

@costin-zaharia-sonarsource costin-zaharia-sonarsource added Area: C# C# rules related issues. Type: Bug Exceptions and blocking issues during analysis. labels Aug 19, 2021
@costin-zaharia-sonarsource costin-zaharia-sonarsource changed the title Could not load file or assembly 'System.Security.Permissions when building projects with dotnet` Could not load file or assembly 'System.Security.Permissions..' when building projects with dotnet Aug 19, 2021
@pavel-mikula-sonarsource
Copy link
Contributor

Let's just remove the dependency. I took a look at the lightweight implementation I did some time ago:
Lexical analyzer: 150 LOC
Syntax analyzer for LL(1) grammar: 100 LOC
Dictionary/List-based object model & rendering (that we don't need): 250 LOC

@andrei-epure-sonarsource
Copy link
Contributor

andrei-epure-sonarsource commented Sep 3, 2021

Ok, this seems like a feature to replace JSON parsing.

We could even just do a string search in JSON files and not care about the parsing. Do we really need proper JSON parsing for good-enough results? (search a substring - the json attribute we're looking for; eat all spaces after and see what's next - the attribute value).

Later edit: well we also need the context... so we actually need the JSON parsing :(

We can also consider using lightweight-json-parser (https://github.com/toptensoftware/JsonKit):

LWJP is a JSON parser written in C# that has only System as a dependency.

Newtonsoft.JSON https://www.nuget.org/packages/Newtonsoft.Json/ - 1.97 MB
Topten.JsonKit.Lite https://www.nuget.org/packages/Topten.JsonKit.Lite/ - 52.53 KB

@andrei-epure-sonarsource
Copy link
Contributor

@costin-zaharia-sonarsource do you have a repro project uploaded somewhere?

@costin-zaharia-sonarsource
Copy link
Member Author

I'm afraid that replacing a dependency with another will not solve our issue. There will still be problems for users who have common dependencies.

@costin-zaharia-sonarsource
Copy link
Member Author

@costin-zaharia-sonarsource do you have a repro project uploaded somewhere?

Yes, there is one added by the original issue reporter. You can find it here: https://github.com/AquilaSands/sonarcloud-build-fail-repro

@andrei-epure-sonarsource
Copy link
Contributor

I'm afraid that replacing a dependency with another will not solve our issue. There will still be problems for users who have common dependencies.

I'm not sure here.

The culprit here is the fact that Newtonsoft relies on Code Access Security, which stopped being supported in .NET Core and was obsoleted in .NET 5 (docs). It was part of the runtime in .NET Framework, in .NET Core the API was kept but did nothing and in .NET 5 it was removed (from my understanding).

Either way, by using a library with less dependencies, IMO we will reduce the risk of conflict while avoiding adding JSON parsing logic in the analyzer.

@costin-zaharia-sonarsource
Copy link
Member Author

costin-zaharia-sonarsource commented Sep 6, 2021

Indeed, by using a library with fewer dependencies we will reduce the risks.

From another point of view, the compiler will always use the version of the assembly we provide and the one referenced by the client code will be ignored, and, as long as this is happening, we will never be able to have a 100% reliable solution since breaking changes or functional ones can happen in the same library from one version to another.

@andrei-epure-sonarsource
Copy link
Contributor

In the end we've decided, for the moment, to bundle newtonsoft inside the DLL (the MIT license allows for it) and to stop providing the nuget as a dependency.

Mid-term we should add a simple JSON parser inside our analyzer and drop the need for newtonsoft altogether (and we'll save some hundreds of KBs in DLL size).

@costin-zaharia-sonarsource
Copy link
Member Author

This looks good to me. Repackaging the dependency will avoid conflicts with other user-added dependencies.

@andrei-epure-sonarsource andrei-epure-sonarsource removed their assignment Sep 13, 2021
@andrei-epure-sonarsource
Copy link
Contributor

Back in TODO - see the comments on #4878 which explain why doing the ILMerge would add complexity to the build and given that JSON parsing is already under way (#4888) it's better to avoid adding the complexity in the build system just for a few weeks.

@andrei-epure-sonarsource
Copy link
Contributor

Coming back on this - #4888 will fix the Newtonsoft problem, but it won't fix conflicts with Google.protobuf. So we'll still need to ILMerge the protobuf library, right @costin-zaharia-sonarsource @pavel-mikula-sonarsource ?

@pavel-mikula-sonarsource
Copy link
Contributor

In theory yes, but do we have a problem with Google.Protobuf? Newtonsoft is IMO a known dependency hell troublemaker while Google.Protobuf has almost no dependencies.

@andrei-epure-sonarsource
Copy link
Contributor

andrei-epure-sonarsource commented Sep 27, 2021

@pavel-mikula-sonarsource we'll be able to answer after the SONARSEC-2690 investigation.

@costin-zaharia-sonarsource
Copy link
Member Author

Theoretically, as long as we have dependencies (even to our packages) we let the door open for this kind of issues. In practice, the chance is not that high for Google.Protobuf (no problems reported so far) so I guess the question is where we can draw a line and be happy with it.

Personally, I would prefer to embed Google.Protobuf to be on the safe side.

@andrei-epure-sonarsource
Copy link
Contributor

see page 3 in this doc - “The Linking Debate” on being a derivative work of the OSS library or not depending on the linking strategy - we'll need to carefully read the software licenses

@andrei-epure-sonarsource
Copy link
Contributor

Fixed in #5049

@Harishapc
Copy link

today i had same issue
but just change player configure Api compatibility level->.NETFRAMEWORK all will be solved in unity
unity->Edit->Project Settings->Player->configure Api compatibility level->.NETFRAMEWORK
thanks me later

@andrei-epure-sonarsource
Copy link
Contributor

Hi @Harishapc - your feedback is valuable - you can open a topic on https://community.sonarsource.com/ and say the solution that you have found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Type: Bug Exceptions and blocking issues during analysis.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants