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.

Sign up for GitHub

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

Jump to bottom

ReleaseDate is not validated as a date. #1725

Open
jedieaston opened this issue Nov 18, 2021 · 7 comments · May be fixed by #2359
Open

ReleaseDate is not validated as a date. #1725

jedieaston opened this issue Nov 18, 2021 · 7 comments · May be fixed by #2359
Assignees
@florelis
Labels
Breaking-Change This is potentially a breaking change for a future major version In-PR Issue related to a PR Issue-Bug It either shouldn't be doing this or needs an investigation.
Milestone
v.Next-Client

Comments

@jedieaston
Copy link
Contributor

Brief description of your issue

In schema v1.1, the ReleaseDate field was added, and it's using the proper JSON Schema syntax to signify it is a date. However, winget validate does not check whether the value is a date, instead just verifying it is a string.

I believe this is because Valijson has no support for date/datetime verification, at least that I could find in their repo.

Steps to reproduce

  1. Make a 1.1 manifest with a ReleaseDate field (singleton and installer types both have the field, so you have options).
  2. Put something that is not a ISO 8601 (YYYY-MM-DD) date in there.
  3. Run winget validate <path to manifest folder>.

Here is an example:

# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.1.0.schema.json
PackageIdentifier: Microsoft.VisualStudio.2022.BuildTools
PackageVersion: 17.0.1
PackageName: Visual Studio Build Tools 2022 Current
Publisher: Microsoft
License: Copyright (c) Microsoft Corporation. All rights reserved.
LicenseUrl: https://visualstudio.microsoft.com/license-terms/
Tags:
- C++
- tools
- C#
- build
- vs
Moniker: vs2022-buildtools
ReleaseDate: "Uh, last Tuesday I think?"
ShortDescription: These Build Tools allow you to build Visual Studio projects from a command-line interface. Use of this tool requires a valid Visual Studio license.
PackageUrl: https://visualstudio.microsoft.com/
Installers:
- Architecture: x64
  InstallerUrl: https://download.visualstudio.microsoft.com/download/pr/8cea3871-c742-43fb-bf8b-8da0699ab4af/25c54d66f5cf07b14cdc0d6dab2e3d5da7ec22dead4757e69011bb2b2946e384/vs_BuildTools.exe
  InstallerSha256: 25C54D66F5CF07B14CDC0D6DAB2E3D5DA7EC22DEAD4757E69011BB2B2946E384
  InstallerType: exe
  InstallerSwitches:
    Silent: --quiet
    SilentWithProgress: --passive
    Custom: --wait
    Upgrade: update
PackageLocale: en-US
ManifestType: singleton
ManifestVersion: 1.1.0

Expected behavior

Manifest validation failed.

ReleaseDate field must be a ISO 8601 (YYYY-MM-DD) date.

Actual behavior

Manifest validation succeeded.

Environment

Windows Package Manager v1.1.12986 (also verified on current dev build)
Windows: Windows.Desktop v10.0.22000.318
Package: Microsoft.DesktopAppInstaller v1.16.12986.0

Any additional software? I have installed Rover the Dog from Windows XP to search through my files, although I don't know what his opinion is on ISO 8601 dates.
@ghost ghost added the Needs-Triage Issue need to be triaged label Nov 18, 2021
This was referenced Nov 18, 2021
Merged
Closed
@denelon denelon added Issue-Bug It either shouldn't be doing this or needs an investigation. and removed Needs-Triage Issue need to be triaged labels Nov 18, 2021
@jedieaston
Copy link
Contributor Author

Found this: tristanpenman/valijson#98 which says that Valijson doesn't support the "format" parts of the JSON schema specs because they are ill-defined. We may need to do this ourselves.

@denelon
Copy link
Contributor

denelon commented Feb 18, 2022

I had similar findings. We may need to implement a semantic validation for the date format.

@denelon denelon added this to the v1.3-Client milestone Feb 22, 2022
@denelon denelon modified the milestones: v1.3-Client, v1.4-Client May 31, 2022
@tristanpenman
Copy link

Just saw that there was a link to this issue in one of the issues raised on the valijson repo. There is a PR that has just been merged which partially implements the format constraint. It covers the built in 'date', 'time' and 'date-time' formats.

Hopefully this will be of some use to you. 🤞

@jedieaston
Copy link
Contributor Author

That does look very useful, thanks for letting us know! Do you have an idea of when a release will be cut for Valijson with that support? Whenever that happens I can looking into updating it here.

@tristanpenman
Copy link

No problem. I've just cut a new release: https://github.com/tristanpenman/valijson/releases/tag/v0.7

@jedieaston jedieaston linked a pull request Jul 22, 2022 that will close this issue
Open
2 tasks
@ghost ghost added the In-PR Issue related to a PR label Jul 22, 2022
@ChristianGalla
Copy link

If the ReleaseDate is validated, it should also be validated, if the date is in a valid range.
For example, a ReleaseDate in the future should be blocked.

Because there is no time zone component, I suggest 1 day offset in the future should be ignored.
For example, if it is already Tuesday local time in Japan, but still Monday in Europe and the US, a Japanese developer should be able to submit a new release created on the same day.

@denelon denelon modified the milestones: v1.4-Client, v1.5-Client Jan 5, 2023
@denelon denelon modified the milestones: v1.5-Client, v.Next-Client Jun 6, 2023
@Trenly
Copy link
Contributor

Trenly commented Jul 25, 2023

[Policy] Breaking Change

@microsoft-github-policy-service microsoft-github-policy-service bot added the Breaking-Change This is potentially a breaking change for a future major version label Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@florelis florelis

Labels
Breaking-Change This is potentially a breaking change for a future major version In-PR Issue related to a PR Issue-Bug It either shouldn't be doing this or needs an investigation.
Projects
None yet
Milestone
v.Next-Client
Development

Successfully merging a pull request may close this issue.

Updated Valijson to version 0.7, adding date validation for manifests. jedieaston/winget-cli
6 participants
@tristanpenman @ChristianGalla @Trenly @florelis @jedieaston @denelon