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

ConvertFrom-Json and Test-Json accept non-compliant/non-standard JSON - no way to strictly validate JSON #10628

Open
chriskuech opened this issue Sep 26, 2019 · 19 comments
Labels
Issue-Enhancement the issue is more of a feature request than a bug Waiting - DotNetCore waiting on a fix/change in .NET WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module

Comments

@chriskuech
Copy link

Summary of the new feature/enhancement

There does not seem to be any way to validate whether a JSON string is valid, which makes it impossible to use PowerShell to validate JSON before passing it to another application.

For example, the following JSON is invalid due to the trailing comma, but Test-Json still returns $true.

'{"hello": "world",}' | Test-Json

I propose adding a -Strict parameter to Test-Json for testing that the JSON string adheres strictly to the JSON specification.

Side note

I was very excited when I found the Test-Json cmdlet, but so far every use case it would have helped, it has not behaved as expected. See also #9560. If this cannot be implemented using the current library, please strongly consider using Json.NET instead of NJsonSchema.

@chriskuech chriskuech added the Issue-Enhancement the issue is more of a feature request than a bug label Sep 26, 2019
@iSazonov
Copy link
Collaborator

@chriskuech We is as simple as possible in Test-Json and delegate all to Json.NET. (NJsonSchema is used only if Schema parameter presents.) We call JObject.Parse(Json). For your example it returns true. So it is Json.NET issue, we can do nothing in the PowerShell Core repo. You could open issue in Json.NET repository.

@iSazonov iSazonov added the Resolution-External The issue is caused by external component(s). label Sep 26, 2019
@chriskuech
Copy link
Author

That makes sense, but I think it also makes sense for the default behavior of PowerShell to have lenient JSON parsing. Because the -Strict parameter provides additional validation, perhaps it makes more sense to do a regex matching.

@iSazonov
Copy link
Collaborator

PowerShell to have lenient JSON parsing

This is not a PowerShell feature to parse third-party languages. It can use features of .Net Core or another library like Json.NET. Moreover, JSON is too complicated for regex.

@KalleOlaviNiemitalo
Copy link

Moreover, JSON is too complicated for regex.

Ugh, nested arrays seem doable with balancing group definitions, but I can't see how to validate the closing braces of nested objects at the same time.

@mklement0
Copy link
Contributor

mklement0 commented Sep 27, 2019

A -Strict mode sounds like a useful addition, to not just check for extraneous trailing commas, but also for the presence of // ... comments, which are also accepted by default.

Trying to roll our own strict mode seems like the wrong way to go, though, as @iSazonov states.

Unfortunately, the Json.Net author in the past has expressed no interest in (opt-in) strict parsing; see this closed issue from 2017: JamesNK/Newtonsoft.Json#1162

@KalleOlaviNiemitalo
Copy link

Would the strict validator be required to reject:

  • duplicate names in JSON objects, as in {"a":1,"a":1}? RFC 8259 section 4 says: "The names within an object SHOULD be unique."
  • huge numbers that match the JSON syntax but are probably not supported by most implementations, such as 1E100000000?

If it is not required to reject those things, then it is simpler to implement and faster to run, I think.

@iSazonov
Copy link
Collaborator

.Net Core team actively works on new JSON API implementation to replace Json.Net. All new features will be in .Net Core. Open feature request issues in CoreFX repo if you want more features - we will get them.

@KalleOlaviNiemitalo
Copy link

System.Text.Json currently seems focused on UTF-8 byte sequences. It seems a bit wasteful if TestJsonCommand has to convert string Json to UTF-8 and have System.Text.Json decode it back. OTOH, perhaps TestJsonCommand has enough overhead elsewhere to make that not matter.

@iSazonov
Copy link
Collaborator

@KalleOlaviNiemitalo It is a question for .Net Core team too.
(Although I don't see performance issues because this cmdlet is mainly used for checking configuration files and not for analyzing high-load Web server streams)

@ghost
Copy link

ghost commented Sep 28, 2019

This issue has been marked as external and has not had any activity for 1 day. It has been be closed for housekeeping purposes.

@asifma
Copy link

asifma commented Oct 6, 2020

When are you planning to solve this?! E.g. I want to validate my Azure ARM-Templates (json) using Test-Json
But it allows trailing commas. Which would not be a valid json syntax....
Guess I can't use it as for now =/

@iSazonov iSazonov added Waiting - DotNetCore waiting on a fix/change in .NET WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module and removed Resolution-External The issue is caused by external component(s). labels Oct 7, 2020
@iSazonov
Copy link
Collaborator

iSazonov commented Oct 7, 2020

I re-open so that we could look this after moving to new .Net Json API.

If you have an interest you could create a simple C# app to test new API and report to .Net Runtime repo. This will increase our chances of getting a solution faster.

@iSazonov
Copy link
Collaborator

#11397 will resolve this.

@rjmholt rjmholt changed the title No way to truly validate JSON ConvertFrom-Json and Test-Json accept non-compliant/non-standard JSON - no way to strictly validate JSON Dec 11, 2020
@cloud-devops-ninja
Copy link

Can anyone update the status of this issue as it is unclear to me whether the above mentioned #11397 resolved the issue as currently our Test-Json is still showing True for JSON that contains a trailing comma. (PowerShell 7.2.6 latest)

I need a solid solution to validate JSON input in my DevOps pipelines so any human error can be excluded from the automated processes.

@iSazonov
Copy link
Collaborator

iSazonov commented Sep 6, 2022

For reference #18023

@gregsdennis
Copy link
Contributor

The JSON Schema implementation has been recently updated to JsonSchema.Net and is available in the latest Powershell snapshot. As such, the parser is now powered by System.Text.Json. Any JSON parsing issues should be handled by that. Please retest this to see if it is still a problem.

@robinmalik
Copy link

Hit this today under PowerShell 7.3.8.

@KalleOlaviNiemitalo
Copy link

#18141 was the merged PR for using System.Text.Json. It's included in PowerShell 7.4.0-preview.4. I don't expect that it will be backported to PowerShell 7.3.

@mklement0
Copy link
Contributor

Only Test-Json was switched to System.Text.Json, which means:

  • v7.4 introduced a massively breaking change, where previously accepted nonstandard JSON now fails the Test-Json test.
  • Test-Json is now inconsistent with ConvertFrom-Json, which (fortunately) still accepts the nonstandard JSON.

What I think should have happened - so as to preserve backward compatibility - is the introduction of, say, a -Strict switch to both cmdlets that enforces standard-JSON-only input on an opt-in basis.

The v7.4.0 Test-Json change wasn't motivated by the desire for strict parsing; the latter only happened as a seemingly unanticipated side effect; see:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement the issue is more of a feature request than a bug Waiting - DotNetCore waiting on a fix/change in .NET WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module
Projects
None yet
Development

No branches or pull requests

8 participants