ConvertFrom-Json: comments in top-level arrays are parsed incorrectly (but also are not errors)

aslatter commented 3 years ago

According to #7436 ConvertFrom-Json should support "JSONC"-style comments present in JSON objects without issue.

However this doesn't seem to work if the comments are in a "top level" array - instead the comments are interpreted as strings-elements within the array.

Steps to reproduce

ConvertFrom-Json -NoEnumerate "[`n// foo`n 100, `n /* bar */`n 200]" | ConvertTo-Json -Compress

Expected behavior

Either an error (due to comments not being supported) or:

[100,200]

Actual behavior

[" foo",100," bar ",200]

Environment data

Name                           Value
----                           -----
PSVersion                      7.2.0-preview.2
PSEdition                      Core
GitCommitId                    7.2.0-preview.2
OS                             Linux 4.19.104-microsoft-standard #1 SMP Wed Feb 19 06:37:35 UTC 2020
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

RookTKO commented 3 years ago

@aslatter I noticed that since Powershell 6 the ConvertFrom-Json supports comments that start with two forward slashes and multi-line comments. I went on to test some stuff out and see what would be acceptable in jsonc format to get a better understanding of how Powershell 7 could be interpreting it.

This is valid in jsonc:

I'm curious if because ConvertFrom-Json takes a JavaScript Object Notation (JSON) formatted string and JSON start with an object with key-value pairs inside it that it takes your array and puts it into a global empty key and everything else becomes a value?

aslatter commented 3 years ago

In my (admittedly quick) reading of RFC 7159 (or http://www.json.org) it looks like a JSON document may be any JSON value (array, object, or scalar), not just an object.

JSON

DHowett commented 11 months ago

I just ran into this as well, with PowerShell 7.4.

odegroot commented 5 months ago

Bug still exists as described

mklement0 commented 5 months ago

To nail down the bug, which affects nested arrays too:

ConvertFrom-Json is built on NewtonSoft.Json, and uses it as follows (from what I can tell, NewtonSoft.Json offers no control over how it handles comments, based on the settings available via the JsonSerializer class):

As desired, it ignores comments:
- at the top-level, i.e. before or after the one and only JSON object or array expected as input.
- between properties of an object.
By contrast, it unexpectedly not only preserves comments inside arrays (whether nested or not), but inappropriately converts them to string elements.
- Here are the culprit lines, which neglect to filter out JValue instances whose .Type value is Comment from the results returned from the NewtonSoft.JSON API; blindly using the .Value property causes the content of comments to be returned as string output array elements:
- https://github.com/PowerShell/PowerShell/blob/95b56bea9625f4448ff66287d8f1152486f1c699/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs#L417-L421
- https://github.com/PowerShell/PowerShell/blob/95b56bea9625f4448ff66287d8f1152486f1c699/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/JsonObject.cs#L313-L317

So it looks like the fix is simple as enclosing the above assignments in if (element.Type != Comment) { ... }

PowerShell / PowerShell