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

ConvertTo-Json string escaped handling differs between PS and PSCore #7693

Closed
PlagueHO opened this Issue Sep 3, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@PlagueHO
Copy link

PlagueHO commented Sep 3, 2018

Steps to reproduce

On PowerShell Core 6.0.4

ConvertTo-Json -InputObject @{ 'abc' = "'def'" }

Expected behavior

Windows PowerShell 5.x returns:

{
    "abc":  "\u0027def\u0027"
}

Actual behavior

PowerShell Core 6.0.4 returns:

{
  "abc": "'def'"
}

Environment data

> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      6.0.4
PSEdition                      Core
GitCommitId                    v6.0.4
OS                             Microsoft Windows 10.0.14393
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

This would appear to be because the StringEscapeHandling setting is set to Default in

JsonSerializerSettings jsonSettings = new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.None, MaxDepth = 1024 };

If the StringEscapeHandling setting is set to [Newtonsoft.Json.StringEscapeHandling]::EscapeHtml then the JSON string that is generated matches what is returned by Windows PowerShell.

image

@iSazonov

This comment has been minimized.

Copy link
Collaborator

iSazonov commented Sep 3, 2018

@markekraus Please look the Issue.

Is this a regression and should be fixed before 6.1.0 GA?

/cc @SteveL-MSFT

@mkht

This comment has been minimized.

Copy link

mkht commented Sep 5, 2018

Is not it a destructive change to change this behavior?

How about add parameter to change behavior like below ?

ConvertTo-Json -InputObject @{ 'abc' = "'def'" } -EscapeHandling Default
ConvertTo-Json -InputObject @{ 'abc' = "'def'" } -EscapeHandling EscapeNonAscii
ConvertTo-Json -InputObject @{ 'abc' = "'def'" } -EscapeHandling EscapeHtml
@SteveL-MSFT

This comment has been minimized.

Copy link
Member

SteveL-MSFT commented Sep 5, 2018

This has been the behavior since we changed to using NewtonSoft.JSON from the .NET Framework JSON serializer and would be a breaking change to change it back. @mkht's suggestion to add a parameter would work and give more flexibility to the user. However, are there other settings that should be exposed via the cmdlet?

@mkht

This comment has been minimized.

Copy link

mkht commented Sep 6, 2018

Confirmed NewtonSoft's reference.
I think that there is no other setting that needs to be exposed.
Other handling settings such as DateFormatHandling can be replaced by formatting the properties of the object to be passed in advance.

If you are adding parameters for escape handling, it will be necessary to discuss how to do the default value.
I think it should be [Newtonsoft.Json.StringEscapeHandling]::EscapeHtml. That's because JSON standard requires that quotation marks must be escaped. However, to avoid destructive changes, it should be [Newtonsoft.Json.StringEscapeHandling]::Default.

@mkht

This comment has been minimized.

Copy link

mkht commented Sep 6, 2018

Sorry, I had a misunderstanding.

Latest JSON standard (RFC 8259) requires that the QUOTATION MARK (U+0022) must be escaped.
But, other quotation marks such as RIGHT SINGLE QUOTATION MARK (U+2019) or APOSTROPHE (U+0027) is not required to be.
Now, I think that the default setting should be [Newtonsoft.Json.StringEscapeHandling]::Default.

https://tools.ietf.org/rfc/rfc8259.txt

All Unicode characters may be placed within the quotation marks, except for the characters that MUST be escaped: quotation mark, reverse solidus, and the control characters (U+0000 through U+001F).

@SteveL-MSFT

This comment has been minimized.

Copy link
Member

SteveL-MSFT commented Sep 12, 2018

@PowerShell/powershell-committee reviewed this and agrees that Default should stay as default and we should expose -EscapeHandling as an optional parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.