Suspicious code fragments in System.Text.Json

[rip-mm]

Description

Hello.

I have checked project using the PVS-Studio analyzer. The results of the check revealed a number of suspicious code fragments that I have included in an article. You may be interested in taking a look at them.

It's worth noting that the article contains only the most interesting warnings issued by the analyzer. You can check your project in more detail using the free version of PVS-Studio for open-source projects.

Reproduction Steps

Not required

Expected behavior

Not required

Actual behavior

Not required

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

[huoyaoyuan]

Thank you for help to diagnose the code quality of runtime repository. The linked article is somehow verbose to read, and it doesn't contain links to the source code that we can identify quickly.

The analyzer reports are valid. Some are already fixed (#112090) by inspiration of your article. Some issues are in the test/debug suite which are less severe.

You are welcomed to submit pull requests to fix the issues. Please submit to main branch and we will backport real important fixes to existing releases.

[dotnet-policy-service]

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

[dotnet-policy-service]

This issue has been marked needs-author-action and may be missing some important information.

[jkotas]

#112384 is fixing issues mentioned in the article that has not been fixed yet, except the ones in System.Text.Json.

@dotnet/area-system-text-json Please take a look at the issues in System.Text.Json

[dotnet-policy-service]

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

[eiriktsarpalis]

The JSON-related code fragments cited are the following:

runtime/src/mono/browser/debugger/BrowserDebugProxy/MemberReferenceResolver.cs

Lines 59 to 71 in 7715391

	public async Task<JObject> GetValueFromObject(JToken objRet, CancellationToken token)
	{
	if (objRet["value"]?["className"]?.Value<string>() == "System.Exception")
	{
	if (DotnetObjectId.TryParse(objRet?["value"]?["objectId"]?.Value<string>(), out DotnetObjectId objectId))
	{
	GetMembersResult exceptionObject = await MemberObjectsExplorer.GetTypeMemberValues(context.SdbAgent, objectId, GetObjectCommandOptions.WithProperties | GetObjectCommandOptions.OwnProperties, token);
	var exceptionObjectMessage = exceptionObject.FirstOrDefault(attr => attr["name"].Value<string>().Equals("_message"));
	exceptionObjectMessage["value"]["value"] = objRet["value"]?["className"]?.Value<string>() + ": " + exceptionObjectMessage["value"]?["value"]?.Value<string>();
	return exceptionObjectMessage["value"]?.Value<JObject>();
	}
	return objRet["value"]?.Value<JObject>();
	}

runtime/src/mono/browser/debugger/BrowserDebugProxy/MemberObjectsExplorer.cs

Lines 100 to 112 in 7715391

	private static async Task<JArray> GetRootHiddenChildren(
	MonoSDBHelper sdbHelper,
	JObject root,
	string rootNamePrefix,
	string rootTypeName,
	GetObjectCommandOptions getCommandOptions,
	bool includeStatic,
	CancellationToken token)
	{
	var rootValue = root?["value"] ?? root["get"];
	if (rootValue?["subtype"]?.Value<string>() == "null")
	return new JArray();

runtime/src/mono/browser/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs

Lines 982 to 993 in 7715391

	foreach (JObject frame in orig_callframes.Value["result"]?["value"]?["frames"])
	{
	string function_name = frame["displayName"]?.Value<string>();
	if (function_name != null && !(function_name.StartsWith("Module._mono_wasm", StringComparison.Ordinal) ||
	function_name.StartsWith("Module.mono_wasm", StringComparison.Ordinal) ||
	function_name == "mono_wasm_fire_debugger_agent_message_with_data" ||
	function_name == "_mono_wasm_fire_debugger_agent_message_with_data" ||
	function_name == "(wasmcall)"))
	{
	callFrames.Add(frame);
	}
	}

They use Json.NET DOM types and are part of the browser debugger.

[dotnet-policy-service]

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.