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

Allow obtaining original string value from ConvertFrom-Json returned DateTime #16976

Open
dancojocaru2000 opened this issue Mar 8, 2022 · 15 comments
Labels
Issue-Enhancement the issue is more of a feature request than a bug WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module WG-NeedsReview Needs a review by the labeled Working Group

Comments

@dancojocaru2000
Copy link

dancojocaru2000 commented Mar 8, 2022

Summary of the new feature / enhancement

As a user I want to be able to access the original string value of a converted DateTime so that it can be used in cases where an equivalent ISO 8601 representation would be considered a different value.

I am using an API which returns ISO 8601 strings as IDs. However, PowerShell automatically converts those IDs to DateTime, and using something like Get-Date -Format o $date returns a different string that results in a different, invalid ID.

Proposed technical implementation details (optional)

There are two ways I can see this implemented:

  1. A -NoDateConversion flag to Convert-FromJson and cmdlets that use it such as Invoke-RestMethod
  2. The returned DateTime could have an OriginalString member, so that the original string can be accessed like $date.OriginalString
  3. The returned DateTime, when passed to Get-Date -Format o $date, would output the original string instead of a ISO 8601 equivalent reinterpretation.
@dancojocaru2000 dancojocaru2000 added Issue-Enhancement the issue is more of a feature request than a bug Needs-Triage The issue is new and needs to be triaged by a work group. labels Mar 8, 2022
@dwtaber
Copy link
Contributor

dwtaber commented Mar 9, 2022

Maybe this concept should be applied to other types the cmdlet converts from strings as well, rather than making DateTime a special case.

@iSazonov
Copy link
Collaborator

iSazonov commented Mar 9, 2022

Please repro steps and input file example.

@iSazonov iSazonov added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Mar 9, 2022
@dancojocaru2000
Copy link
Author

Maybe this concept should be applied to other types the cmdlet converts from strings as well, rather than making DateTime a special case.

Agreed, DateTime was the first to get me in trouble so that's why I singled it out.

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Mar 9, 2022
@dancojocaru2000 dancojocaru2000 changed the title Allow obtaining original string value from Convert-FromJson returned DateTime Allow obtaining original string value from ConvertFrom-Json returned DateTime Mar 9, 2022
@dancojocaru2000
Copy link
Author

Please repro steps and input file example.

$json = "[{`"id`": `"2022-03-09T09:11:04.254630+00:00`", `"otherData`": true}]"
$converted = ConvertFrom-Json $json
$converted[0].id | Get-Date -Format o

As can be seen, the input string is 2022-03-09T09:11:04.254630+00:00. The output of the last command on my machine is 2022-03-09T11:11:04.2546300+02:00. Since the two strings aren't identical, using the 2nd one as the ID results in an error.

I want to highlight that I don't want to obtain a +00:00 ISO 8601 string! It just so happens that this time the input is +00:00. If the input would be 2022-03-09T11:11:04.2546300+02:00 and the output would be 2022-03-09T09:11:04.254630+00:00, it would still be an error.

@iSazonov
Copy link
Collaborator

iSazonov commented Mar 9, 2022

I see ConvertFrom-Json converts the string to local datetime. It is how underlying NewtonSoft.NET works. So we can not change this. You have to do some manual steps to convert local datetime to string you need.

@dancojocaru2000
Copy link
Author

dancojocaru2000 commented Mar 9, 2022

Is it not possible to somehow disable the automatic conversions and keep all strings as strings? Because this seems as a flaw to me (that should be perhaps fixed in NewtonSoft.NET), destructively converting data without an option to get the original back.

Edit: I found this Stack Overflow question: Json.NET Disable the deserialization on DateTime. Would it not possible to pass such options?

@iSazonov
Copy link
Collaborator

iSazonov commented Mar 9, 2022

Edit: I found this Stack Overflow question: Json.NET Disable the deserialization on DateTime. Would it not possible to pass such options?

No, it will be huge breaking change.

Is your source string always in UTC? If yes, you could use an workaround otherwise it is a dead end.

@dancojocaru2000
Copy link
Author

dancojocaru2000 commented Mar 9, 2022

Why would it be a breaking change?

And no, the source string is not always following a certain format.

@iSazonov
Copy link
Collaborator

iSazonov commented Mar 9, 2022

Why would it be a breaking change?

The cmdlet will return different result.

And no, the source string is not always following a certain format.

In the case you could pre-process the json so that "hide/mask" the datetime string.

@dancojocaru2000
Copy link
Author

Why would it be a breaking change?

The cmdlet will return different result.

There could be a -NoConversion flag which would be a backwards compatible change.

@jborean93
Copy link
Collaborator

jborean93 commented Mar 10, 2022

This is definitely possible to do without a breaking change. You can have an opt in parameter like -DateTimeFormat None|DateTime|DateTimeOffset that controls the DateParseHandling property in Newtonsoft. The default can be DateTime to preserve backwards compatibility if that is desired.

PowerShell already

new JsonSerializerSettings
{
// This TypeNameHandling setting is required to be secure.
TypeNameHandling = TypeNameHandling.None,
MetadataPropertyHandling = MetadataPropertyHandling.Ignore,
MaxDepth = maxDepth
});
passes in a settings class so it shouldn't be that hard to add. The only concern I would have is that it would tie ConvertFrom-Json more closely to Newtonsoft making it harder to potentially migrate to another JSON library in the future (if that's ever going to happen).

The current DateTime handling is a bit subpar because it always gives you a local kind DateTime object where you can loose information about the value like what TZ it was in. For example my local offset is +10:00 and this is what I see

$json = '"2022-03-09T09:11:04.25463+05:00"'
$converted = ConvertFrom-Json $json

$converted

# Wednesday, 9 March 2022 2:11:04 pm

$converted.Kind
# Local

I have no way to know with this deserialized value that the raw string had a +05:00 offset, I just know what the value is in UTC and local time.

@SydneyhSmith SydneyhSmith added the WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module label May 16, 2022
Copy link
Contributor

This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Resolution-No Activity Issue has had no activity for 6 months or more label Nov 15, 2023
Copy link
Contributor

This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.

@kilasuit kilasuit reopened this Dec 5, 2023
@kilasuit kilasuit added WG-NeedsReview Needs a review by the labeled Working Group and removed Needs-Triage The issue is new and needs to be triaged by a work group. Resolution-No Activity Issue has had no activity for 6 months or more labels Dec 5, 2023
@kilasuit
Copy link
Collaborator

kilasuit commented Dec 5, 2023

Reopening this for WG to review as I do think this is a bug as there shouldn't be a conversion unless the user requests one, which may deviate from current experience but would be reasonable to change going forward.

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 WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module WG-NeedsReview Needs a review by the labeled Working Group
Projects
None yet
Development

No branches or pull requests

7 participants