Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix Test-Json not handling non-object types at root#17741

Merged
iSazonov merged 2 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
dkaszews:dkaszews-test-json-root-not-objectCopy head branch name to clipboard
Jul 25, 2022
Merged

Fix Test-Json not handling non-object types at root#17741
iSazonov merged 2 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
dkaszews:dkaszews-test-json-root-not-objectCopy head branch name to clipboard

Conversation

@dkaszews

@dkaszews dkaszews commented Jul 21, 2022

Copy link
Copy Markdown
Contributor

PR Summary

Test-Json incorrectly parsed JSON as JObject, which failed if the root type was not an object, but array, string, number, etc. @mklement0 suggested to change it to JToken which matches all types, which I verified to be a one-line fix.

Fixes #11384

PR Context

Test-Json currently fails if the root type is not an object, despite ConvertFrom-Json and ConvertTo-Json handling them without a problem. This leads to confusing case of 1 | ConvertTo-Json | Test-Json failing with a non-specific error "Cannot parse JSON".

This bug is most apparent when trying to validate a JSON schema where the root is not a single object, but instead an array of objects. However, as in example above, it also impacts cases without schema, where someone might use Test-Json as a shorthand for putting ConvertFrom-Json in a try-catch, which is exactly how it should be used when you don't care about content, just whether it is valid.

PR Checklist

@dkaszews dkaszews requested a review from PaulHigin as a code owner July 21, 2022 22:04
@ghost ghost assigned TravisEz13 Jul 21, 2022
@ghost

ghost commented Jul 21, 2022

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Test-Json.Tests.ps1 Outdated
Co-authored-by: Ilya <darpa@yandex.ru>
@dkaszews dkaszews requested a review from iSazonov July 22, 2022 06:47

@daxian-dbw daxian-dbw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jul 22, 2022
@iSazonov iSazonov assigned iSazonov and unassigned TravisEz13 Jul 25, 2022
@iSazonov iSazonov merged commit 89cc6d1 into PowerShell:master Jul 25, 2022
@iSazonov

Copy link
Copy Markdown
Collaborator

@dkaszews Thanks for your contribution!

@ghost

ghost commented Aug 12, 2022

Copy link
Copy Markdown

🎉v7.3.0-preview.7 has been released which incorporates this pull request.:tada:

Handy links:

@TravisEz13 TravisEz13 mentioned this pull request Sep 30, 2022
22 tasks
RobertKlohr referenced this pull request in RobertKlohr/PowerShellForLockpath Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test-Json doesn't recognize JSON arrays or primitives

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.