Skip to content

Don't try to parse JSON strings as date#1492

Merged
gregg-miskelly merged 2 commits intomicrosoft:mainfrom
stertingen:dont-parse-date-from-json
Feb 24, 2025
Merged

Don't try to parse JSON strings as date#1492
gregg-miskelly merged 2 commits intomicrosoft:mainfrom
stertingen:dont-parse-date-from-json

Conversation

@stertingen
Copy link
Contributor

@stertingen stertingen commented Feb 21, 2025

When parsing launch options from JSON, the JSON parser's default behavior is – for whatever reason – to parse slightly datetime-ish looking strings as date objects. This PR explicitly forbids this behavior when parsing launch options.

Partially fixes #1491, microsoft/vscode-cpptools#13241 and microsoft/vscode#238514.

@stertingen stertingen closed this Feb 21, 2025
@stertingen
Copy link
Contributor Author

Never mind, this PR does not fix anything. It just modifies the string in a more subtle way.

@stertingen stertingen reopened this Feb 23, 2025
@stertingen
Copy link
Contributor Author

Diving deeper into this issue, I have realized that the arguments are parsed from JSON twice, and both times wrong.

  1. Coming from a sample argument string: "2022-01-03T13:37:42+02:00"
  2. String gets converted to Date by Microsoft.VisualStudio.Shared.VSCodeDebugProtocol (see Microsoft.VisualStudio.Shared.VSCodeDebugProtocol should not parse ISO 8601 compliant strings as date object VSDebugAdapterHost#34)
  3. Date is serialized to string "2022-01-03T12:37:42+01:00" in
    string launchJson = JsonConvert.SerializeObject(responder.Arguments.ConfigurationProperties);
  4. Date is parsed again as Date in
    JObject parsedOptions = JObject.Parse(options);
  5. Date is serialized to another string "01/03/2022 12:37:42" in
    baseOptions = parsedJObject.ToObject<Json.LaunchOptions.LaunchOptions>();

This PR fixes the second time the JSON document is parsed, prevents parsing as a date in step 4.

@gregg-miskelly
Copy link
Member

Thanks for working so hard to fix this!

Copy link
Member

@gregg-miskelly gregg-miskelly left a comment

Choose a reason for hiding this comment

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

Aside from that, looks good to me

Apply suggestion from greg-miskelly

Co-authored-by: Gregg Miskelly <greggm@microsoft.com>
@gregg-miskelly gregg-miskelly merged commit 8dae539 into microsoft:main Feb 24, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Debugging from VS code rewrites args if containing ISO 8601 compliant date/time string

2 participants