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

Test-Json string format not matching #19888

Open
5 tasks done
FormindAPE opened this issue Jul 4, 2023 · 20 comments
Open
5 tasks done

Test-Json string format not matching #19888

FormindAPE opened this issue Jul 4, 2023 · 20 comments
Labels
Needs-Triage The issue is new and needs to be triaged by a work group. WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module

Comments

@FormindAPE
Copy link

FormindAPE commented Jul 4, 2023

Prerequisites

Steps to reproduce

Let's create a very simple json schema that use the string format duration :

{
	"type": "object",
	"additionalProperties":false,
	"properties": {
		"duration": {
			"type": "string",
			"format": "duration"
		}
	}
}

the format duration was added in the json schema "draft 2019-09" current version is "draft 2020-12". The goal of the duration format is to match ISO 8601 (see: json schema documentation )

and save it in a file named schema.json

and run the following command to test:

> Test-Json -SchemaFile .\schema.json -Json '{"duration":"test123"}'

or

> Test-Json -SchemaFile .\schema.json -Json '{"duration":"PT1D"}'

the result will be True when expecting False

Expected behavior

PS> Test-Json -SchemaFile .\schema.json -Json '{"duration":"test123"}'
False
PS> Test-Json -SchemaFile .\schema.json -Json '{"duration":"PT1D"}'
False


PS> Test-Json -SchemaFile .\schema.json -Json '{"duration":"P1D"}'
True

Actual behavior

PS> Test-Json -SchemaFile .\schema.json -Json '{"duration":"test123"}'
True
PS> Test-Json -SchemaFile .\schema.json -Json '{"duration":"PT1D"}'
True


PS> Test-Json -SchemaFile .\schema.json -Json '{"duration":"P1D"}'
True

Error details

No response

Environment data

Name                           Value
----                           -----
PSVersion                      7.4.0-preview.4
PSEdition                      Core
GitCommitId                    7.4.0-preview.4
OS                             Microsoft Windows 10.0.22621
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visuals

No response

@FormindAPE FormindAPE added the Needs-Triage The issue is new and needs to be triaged by a work group. label Jul 4, 2023
@kilasuit kilasuit added the WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module label Jul 4, 2023
@brian6932
Copy link

brian6932 commented Jul 5, 2023

There seems to be an issue regarding sending " within strings to programs that are not native pwsh, they aren't getting escaped in 7.4.0-preview.4, no matter the string type you use ""...""and '"..."' will result in the same thing

@FormindAPE
Copy link
Author

FormindAPE commented Jul 7, 2023

the same behavior happens when piping a file to the command:

> Get-content .\schema.json
{
        "type": "object",
        "additionalProperties":false,
        "properties": {
                "duration": {
                        "type": "string",
                        "format": "duration"
                }
        }
}
> Get-content .\data.json
{"duration":"test123"}
> Get-content .\data.json | Test-Json -SchemaFile .\schema.json
True

@brian6932
Copy link

Can you please revise the title of this issue, as it's a bit too specific, it'll likely cause many copies of it

@FormindAPE
Copy link
Author

There seems to be an issue regarding sending " within strings to programs that are not native pwsh, they aren't getting escaped in 7.4.0-preview.4, no matter the string type you use ""...""and '"..."' will result in the same thing

@brian6932
I got 2 different behavior when using '"..."' and ""..."" (see below) so I think it's not related

PS > echo "{"duration":"test"}" | Test-Json -SchemaFile .\schema.json        
Test-Json: Cannot parse the JSON.
False
Test-Json: Cannot parse the JSON.
False
PS > echo '{"duration":"test"}' | Test-Json -SchemaFile .\schema.json        
True

@FormindAPE
Copy link
Author

FormindAPE commented Jul 7, 2023

After some more digging I have noticed that none of the json shema string format options are working in 7.4.0-preview.4
but that in 7.3.5 moste of the string format options were working (only some option like duration or regex were not woking)

@FormindAPE FormindAPE changed the title Test-Json not matching string format "duration" Test-Json string format not matching Jul 7, 2023
@iSazonov
Copy link
Collaborator

iSazonov commented Jul 8, 2023

/cc @gregsdennis

@gregsdennis
Copy link
Contributor

gregsdennis commented Jul 8, 2023

In JSON Schema, format is an annotation by default. That means it doesn't validate. So the current behavior is actually more correct, per the spec.

The two solutions I see here are:

  1. Just internally configure JsonSchema.Net to validate format by default. This may align with expected behavior, but you wouldn't be able to turn it off.
  2. Add a new PowerShell parameter that enables that configuration, e.g. -ValidateFormat.

The fix is pretty easy; we just need to decide which to do.

@iSazonov
Copy link
Collaborator

iSazonov commented Jul 8, 2023

@gregsdennis Thanks! I hope WG will do a conclusion. /cc @SteveL-MSFT

@brian6932
Copy link

brian6932 commented Sep 28, 2023

7.4.0-preview.6 fixed the issue I had

@gregsdennis
Copy link
Contributor

Uh... what was the fix? Where was it implemented (e.g. a PR)?

@brian6932
Copy link

brian6932 commented Sep 28, 2023

Not sure, but I don't have any issues with nesting quote types anymore '{"...":"..."}' seems to parse just find now 🤷

@gregsdennis
Copy link
Contributor

Oh, so your problem is with quotes in general, which isn't what this issue is about.

@brian6932
Copy link

It seemed like the same underlying cause

@TobiasPSP
Copy link
Collaborator

Is there still the need for a fix/WG discussion, or was is just a quoting issue?

@pearj
Copy link

pearj commented Jan 16, 2024

Is there still the need for a fix/WG discussion, or was is just a quoting issue?

@TobiasPSP This is still definitely a problem, and the quotes issue @brian6932 was experiencing was a red herring I believe.

After some more digging I have noticed that none of the json shema string format options are working in 7.4.0-preview.4
but that in 7.3.5 moste of the string format options were working (only some option like duration or regex were not woking)

I agree with @FormindAPE that it was mostly working in 7.3.x, I tried 7.3.6 that I had lying around and date-time was working but duration was not.

Using this schema.json:

{
  "type": "object",
  "additionalProperties": false,
  "properties": {
    "duration": {
      "type": "string",
      "format": "duration"
    },
    "datetest": {
      "type": "string",
      "format": "date-time"
    }
  }
}

In PowerShell 7.3.6:

PS > Test-Json -SchemaFile .\schema.json -Json '{"datetest":"test123"}'
Test-Json: DateTimeExpected: #/datetest
False

PS > Test-Json -SchemaFile .\schema.json -Json '{"duration":"test123"}' 
True

In PowerShell 7.4.0:

PS > Test-Json -SchemaFile .\schema.json -Json '{"datetest":"test123"}'
True

PS > Test-Json -SchemaFile .\schema.json -Json '{"duration":"test123"}' 
True

The two solutions I see here are:

  1. Just internally configure JsonSchema.Net to validate format by default. This may align with expected behavior, but you wouldn't be able to turn it off.
  2. Add a new PowerShell parameter that enables that configuration, e.g. -ValidateFormat.

The fix is pretty easy; we just need to decide which to do.

@gregsdennis My vote is for doing 1, in a patch version of PowerShell 7.4.x, since this is essentially a possible unintentional breaking change from 7.3.x. then do 2 in maybe 7.5 or something like that?

@gregsdennis
Copy link
Contributor

I don't mind doing both of these as a single update, but I would need someone from the PS team to confirm.

@gregsdennis
Copy link
Contributor

cc: @iSazonov ☝️

@iSazonov
Copy link
Collaborator

From document referenced above:

A JSON Schema validator will ignore any format type that it does not understand.

So I think it is optional feature for the cmdlet too. In any case we need WG conclusion.

@gregsdennis
Copy link
Contributor

gregsdennis commented Jan 22, 2024

From document referenced above:

A JSON Schema validator will ignore any format type that it does not understand. - @iSazonov

To be clear, this isn't about unknown formats, it's about validating format at all.

The docs:

By default, format is just an annotation and does not effect validation.

The spec requires that format is an annotation unless the implementation is configured to validate or the format-assertion vocabulary is specified by the meta-schema.

Since, generally, the default meta-schemas do not reference the format-assertion vocab, our case falls to the configuration of the lib.

@pearj is arguing that previous versions of PowerShell validated format, implying that such configuration was enabled by default, and that format now not being validated is a breaking change which should be reverted (enable format validation by default).

I'm happy to have validation enabled by default, if that's what PowerShell wants to do, but it needs to have a switch to disable it. (Personally I feel this is backwards, but I understand the historical context.)

@pearj
Copy link

pearj commented Jan 22, 2024

Personally I feel this is backwards, but I understand the historical context

@gregsdennis Yes you are right, going to the WG sounded like it could take a long time, so I was hoping to restore functionality in a quicker timeframe. But if either change requires WG approval then off by default probably makes the most sense then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Triage The issue is new and needs to be triaged by a work group. WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module
Projects
None yet
Development

No branches or pull requests

7 participants