Fix package pattern in json schema configuration registry (#1429)

.github/workflows/pr.yaml

@@ -70,3 +70,27 @@ jobs:
         with:
           name: format.patch
           path: out/format.patch
+
+  json-schema:
+    runs-on: windows-2022
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@v3
+      - name: Get microsoft/vcpkg pinned sha into VCPKG_SHA
+        id: vcpkg_sha
+        shell: pwsh
+        run: |
+          "VCPKG_SHA="+(Get-Content vcpkg-init/vcpkg-scripts-sha.txt -Raw).Trim() >> $env:GITHUB_OUTPUT
+      - name: Checkout microsoft/vcpkg for end-to-end tests
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+          path: ${{ github.workspace }}/vcpkg-root
+          repository: microsoft/vcpkg
+          ref: ${{ steps.vcpkg_sha.outputs.VCPKG_SHA }}
+      - name: Run vcpkg json-schema end-to-end tests
+        shell: pwsh
+        run: |
+          ${{ github.workspace }}/azure-pipelines/json-schema-tests.ps1
+        env:
+          VCPKG_ROOT: ${{ github.workspace }}/vcpkg-root

azure-pipelines/json-schema-tests-dir/json-schema-bvt.test.ps1

@@ -0,0 +1,117 @@
+
+function GenPackageJson {
+    param(
+        [Parameter(Mandatory)][bool]$Expected,
+        [Parameter(Mandatory)][AllowEmptyString()][string[]]$PackageArray,
+        [string]$Baseline = '0' * 40
+    )
+    return @(
+        $Expected,
+        @{
+            registries = @(
+                [pscustomobject]@{
+                    kind       = 'git'
+                    repository = ''
+                    baseline   = $Baseline
+                    packages   = $PackageArray
+                }
+            )
+        }
+    )
+}
+
+# See src/vcpkg-test/registries.cpp "check valid package patterns"
+@{
+    $VcpkgJsonSchema.Configuration =
+    @{
+        'packages: ["a"]'                 = GenPackageJson $true  @('a')
+        'packages: empty'                 = GenPackageJson $false [string[]]@('')
+        'packages: blank'                 = GenPackageJson $false @(' ')
+        'packages: baseline 39'           = GenPackageJson $false @('a') ('0' * 39)
+        'packages: hashtag ["*"]'         = GenPackageJson $true  @('*')
+        'packages: hashtag ["a*"]'        = GenPackageJson $true  @('a*')
+        'packages: hashtag ["*a"]'        = GenPackageJson $false @('*a')
+        'packages: hashtag ["b-*"]'       = GenPackageJson $true  @('b-*')
+        'packages: hashtag ["c-d-*"]'     = GenPackageJson $true  @('c-d-*')
+        'packages: hashtag dup ["a**"]'   = GenPackageJson $false @('a**')
+        'packages: hashtag dup ["b-**"]'  = GenPackageJson $false @('b-**')
+        'packages: hashtag dup ["c--*"]'  = GenPackageJson $false @('c--*')
+        'packages: hashtag dup ["d-*-*"]' = GenPackageJson $false @('d-*-*')
+        'packages: hashtag mid ["a*b"]'   = GenPackageJson $false @('a*b')
+        'packages: mix array ["a*","b"]'  = GenPackageJson $true  @('a*', 'b')
+        'packages: symbols ["a+"]'        = GenPackageJson $false @('a+')
+        'packages: symbols ["a?"]'        = GenPackageJson $false @('a?')
+    }
+    $VcpkgJsonSchema.Port          =
+    @{
+        # test identifiers
+        'port-name: "co"'                 = $true, @{name = 'co' }
+        'port-name: "rapidjson"'          = $true, @{name = 'rapidjson' }
+        'port-name: "boost-tuple"'        = $true, @{name = 'boost-tuple' }
+        'port-name: "vcpkg-boost-helper"' = $true, @{name = 'vcpkg-boost-helper' }
+        'port-name: "lpt"'                = $true, @{name = 'lpt' }
+        'port-name: "com"'                = $true, @{name = 'com' }
+        # reject invalid characters
+        'port-name: ""'                   = $false, @{name = '' }
+        'port-name: " "'                  = $false, @{name = ' ' }
+        'port-name: "boost_tuple"'        = $false, @{name = 'boost_tuple' }
+        'port-name: "boost.'              = $false, @{name = 'boost.' }
+        'port-name: "boost.tuple"'        = $false, @{name = 'boost.tuple' }
+        'port-name: "boost@1"'            = $false, @{name = 'boost@1' }
+        'port-name: "boost#1"'            = $false, @{name = 'boost#1' }
+        'port-name: "boost:x64-windows"'  = $false, @{name = 'boost:x64-windows' }
+        # accept legacy
+        'port-name: "all_modules"'        = $false, @{name = 'all_modules' } # removed in json-schema
+        # reject reserved keywords
+        'port-name: "prn"'                = $false, @{name = 'prn' }
+        'port-name: "aux"'                = $false, @{name = 'aux' }
+        'port-name: "nul"'                = $false, @{name = 'nul' }
+        'port-name: "con"'                = $false, @{name = 'con' }
+        'port-name: "core"'               = $false, @{name = 'core' }
+        'port-name: "default"'            = $false, @{name = 'default' }
+        'port-name: "lpt0"'               = $false, @{name = 'lpt0' }
+        'port-name: "lpt9"'               = $false, @{name = 'lpt9' }
+        'port-name: "com0"'               = $false, @{name = 'com0' }
+        'port-name: "com9"'               = $false, @{name = 'com9' }
+        # reject incomplete segments
+        'port-name: "-a"'                 = $false, @{name = '-a' }
+        'port-name: "a-"'                 = $false, @{name = 'a-' }
+        'port-name: "a--"'                = $false, @{name = 'a--' }
+        'port-name: "---"'                = $false, @{name = '---' }
+    }
+}.GetEnumerator()
+| ForEach-Object {
+    @{
+        SchemaName = $_.Key
+        JsonCases  = $_.Value.GetEnumerator() | ForEach-Object {
+            @{
+                Title    = $_.Key
+                Expected = $_.Value[0]
+                Json     = ConvertTo-Json -InputObject $_.Value[1] -Depth 5 -Compress
+            }
+        }
+    }
+}
+| ForEach-Object {
+    $_SchemaName = $_.SchemaName
+    $_SchemaPath = Join-Path $WorkingRoot $_SchemaName
+    $_.JsonCases | ForEach-Object {
+        $_Title = $_.Title
+        $_Expected = $_.Expected
+
+        $_Actual = Test-Json -ea:0 -Json $_.Json -SchemaFile $_SchemaPath
+        $_Result = $_.Expected -eq $_Actual ? 'Pass':'Fail'
+        if ($_Result -eq 'Fail') {
+            throw "$_SchemaName validate fail with $_Title, expected $_Expected"
+        }
+        [pscustomobject]@{
+            SchemaName = $_SchemaName
+            Title      = $_Title
+            Expected   = $_Expected
+            Actual     = $_Actual
+            Result     = $_Result
+        }
+    }
+}
+| Sort-Object SchemaName, Title
+| Format-Table

azure-pipelines/json-schema-tests-dir/vcpkg-ports-json.test.ps1

@@ -0,0 +1,19 @@
+
+$VcpkgPortSchemaPath = Join-Path $WorkingRoot $VcpkgJsonSchema.Port
+
+Get-ChildItem -Directory -Path (Join-Path $VcpkgRoot 'ports')
+| ForEach-Object -Parallel {
+    $PortName = $_.Name
+    $PortDir = $_.FullName
+    $PortJsonPath = Join-Path $PortDir 'vcpkg.json'
+    $Schema = $using:VcpkgPortSchemaPath
+
+    $_Actual = Test-Json -ea:0 -LiteralPath $PortJsonPath -SchemaFile $Schema
+    [pscustomobject]@{
+        PortName = $PortName
+        Actual   = $_Actual
+    }
+}
+| ForEach-Object { Write-Host $_; $_ }
+| Where-Object Actual -EQ $false
+| ForEach-Object { Write-Error $_; $_ }

azure-pipelines/json-schema-tests.ps1

@@ -0,0 +1,52 @@
+[CmdletBinding()]
+Param(
+    [ValidateNotNullOrEmpty()]
+    [string]$WorkingRoot = 'work',
+    [Parameter(Mandatory = $false)]
+    [string]$VcpkgRoot
+)
+
+$ErrorActionPreference = 'Stop'
+
+if ($PSVersionTable.PSVersion.Major -lt 7) {
+    Write-Error "json-schema e2e tests must use pwsh"
+}
+
+$VcpkgSrcDir = $PWD
+
+$WorkingRoot = (New-Item -Path $WorkingRoot -ItemType Directory -Force).FullName
+
+$VcpkgRoot = & {
+    if (-not [string]::IsNullOrWhitespace($VcpkgRoot)) {
+        return $VcpkgRoot
+    }
+    if ([string]::IsNullOrWhitespace($env:VCPKG_ROOT)) {
+        throw "Could not determine VCPKG_ROOT"
+    }
+    return $env:VCPKG_ROOT
+} | Get-Item | Select-Object -ExpandProperty FullName
+
+$VcpkgJsonSchema = @{
+    Artifact      = 'artifact.schema.json'
+    Configuration = 'vcpkg-configuration.schema.json'
+    Definitions   = 'vcpkg-schema-definitions.schema.json'
+    Port          = 'vcpkg.schema.json'
+}
+
+# remove `$id` in schema for error 'Test-Json: Cannot parse the JSON schema.'
+$VcpkgJsonSchema.Values
+| ForEach-Object {
+    Copy-Item -Path (Join-path $VcpkgSrcDir 'docs' $_) -Destination $WorkingRoot
+    Join-Path $WorkingRoot $_
+}
+| ForEach-Object {
+    (Get-Content -Raw -Path $_) -replace '(?s)\n  "\$id".+?\n', "`n"
+    | Set-Content -NoNewline -Path $_
+}
+| Out-Null
+
+Get-ChildItem $PSScriptRoot/json-schema-tests-dir/*.test.ps1
+| ForEach-Object {
+    Write-Host "Running test $_"
+    & $_.FullName
+}

docs/artifact.schema.json

@@ -1,4 +1,5 @@
 {
+  "$schema": "http://json-schema.org/draft-07/schema#",
   "$id": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/artifact.schema.json",
   "type": "object",
   "properties": {

docs/vcpkg-configuration.schema.json

@@ -1,4 +1,5 @@
 {
+  "$schema": "http://json-schema.org/draft-07/schema#",
   "$id": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg-configuration.schema.json",
   "type": "object",
   "properties": {

docs/vcpkg-schema-definitions.schema.json

@@ -1,5 +1,5 @@
 {
-  "$schema": "http://json-schema.org/draft-07/schema",
+  "$schema": "http://json-schema.org/draft-07/schema#",
   "$id": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg-schema-definitions.schema.json",
   "definitions": {
     "artifact-references": {
@@ -194,7 +194,7 @@
       "description": "A port dependency fetchable by vcpkg.",
       "oneOf": [
         {
-          "$ref": "#/definitions/port-name"
+          "$ref": "#/definitions/identifier"
         },
         {
           "$ref": "#/definitions/dependency-object"
@@ -206,7 +206,7 @@
       "type": "object",
       "properties": {
         "name": {
-          "$ref": "#/definitions/port-name"
+          "$ref": "#/definitions/identifier"
         },
         "features": {
           "type": "array",
@@ -731,8 +731,7 @@
         },
         {
           "not": {
-            "description": "Identifiers must not be a Windows filesystem or vcpkg reserved name.",
+            "$ref": "#/definitions/reserved-name"
-            "pattern": "^(prn|aux|nul|con|lpt[1-9]|com[1-9]|core|default)$"
           }
         }
       ]
@@ -759,9 +758,9 @@
           "type": "object",
           "$comment": "The regexes below have (#\\d+)? added to the end as overrides allow putting the port-version inline",
           "properties": {
-              "name": {
+            "name": {
-                "$ref": "#/definitions/identifier"
+              "$ref": "#/definitions/identifier"
-              },
+            },
             "version-string": {
               "description": "Deprecated in favor of 'version' in overrides. Text used to identify an arbitrary version",
               "type": "string",
@@ -800,18 +799,18 @@
         }
       ]
     },
-    "port-name": {
+    "package-pattern": {
       "type": "string",
-      "description": "Name of a package.",
+      "description": "A pattern to match package name.",
       "allOf": [
         {
-          "description": "Package name must be a dot-separated list of valid identifiers",
+          "$comment": "https://learn.microsoft.com/en-us/vcpkg/reference/vcpkg-configuration-json#registry-packages",
-          "pattern": "^[a-z0-9]+(-[a-z0-9]+)*(\\.[a-z0-9]+(-[a-z0-9]+)*)*$"
+          "description": "Package pattern may contain only lowercase letters, digits, and -, with an optional trailing *",
+          "pattern": "^([a-z0-9]+-)*([a-z0-9]+[*]?|[*])$"
         },
         {
           "not": {
-            "description": "Identifiers must not be a Windows filesystem or vcpkg reserved name.",
+            "$ref": "#/definitions/reserved-name"
-            "pattern": "(^|\\.)(prn|aux|nul|con|lpt[1-9]|com[1-9]|core|default)(\\.|$)"
           }
         }
       ]
@@ -860,7 +859,7 @@
                     "packages": {
                       "type": "array",
                       "items": {
-                        "$ref": "#/definitions/port-name"
+                        "$ref": "#/definitions/package-pattern"
                       }
                     }
                   },
@@ -887,6 +886,21 @@
         }
       ]
     },
+    "reserved-name": {
+      "description": "Vcpkg reserved identifier names for lowercase.",
+      "type": "string",
+      "anyOf": [
+        {
+          "description": "Vcpkg reserved names.",
+          "pattern": "^(core|default)$"
+        },
+        {
+          "$comment": "https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions",
+          "description": "A relaxed pattern of Win32 filesystem reserved names.",
+          "pattern": "^(con|prn|aux|nul|com[0-9]|lpt[0-9])(\\.[^.]+)*$"
+        }
+      ]
+    },
     "semantic-version": {
       "description": "A semantic version string. See https://semver.org/",
       "type": "string",
@@ -915,7 +929,7 @@
         {
           "not": {
             "description": "Identifiers and parts delimited by slashes must not be a Windows filesystem or vcpkg reserved name.",
-            "pattern": "(^|/)(prn|aux|nul|con|lpt[1-9]|com[1-9]|core|default)(/|$)"
+            "pattern": "(^|/)(prn|aux|nul|con|lpt[0-9]|com[0-9]|core|default)(/|$)"
           }
         }
       ]

docs/vcpkg.schema.json

@@ -1,5 +1,5 @@
 {
-  "$schema": "http://json-schema.org/draft-07/schema",
+  "$schema": "http://json-schema.org/draft-07/schema#",
   "$id": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json",
   "type": "object",
   "allOf": [
@@ -7,7 +7,7 @@
       "properties": {
         "name": {
           "description": "The name of the top-level package",
-          "$ref": "vcpkg-schema-definitions.schema.json#/definitions/port-name"
+          "$ref": "vcpkg-schema-definitions.schema.json#/definitions/identifier"
         },
         "version-string": {
           "description": "Text used to identify an arbitrary version",

include/vcpkg/base/jsonreader.h

@@ -335,7 +335,7 @@ namespace vcpkg::Json
     struct IdentifierDeserializer final : Json::IDeserializer<std::string>
     {
         virtual LocalizedString type_name() const override;
-        // [a-z0-9]+(-[a-z0-9]+)*, plus not any of {prn, aux, nul, con, lpt[1-9], com[1-9], core, default}
+        // [a-z0-9]+(-[a-z0-9]+)*, plus not any of {prn, aux, nul, con, lpt[0-9], com[0-9], core, default}
         static bool is_ident(StringView sv);
         virtual Optional<std::string> visit_string(Json::Reader&, StringView sv) const override;
         static const IdentifierDeserializer instance;

src/vcpkg-test/registries.cpp

@@ -121,11 +121,15 @@ TEST_CASE ("check valid package patterns", "[registries]")
     CHECK(ID::is_ident("rapidjson"));
     CHECK(ID::is_ident("boost-tuple"));
     CHECK(ID::is_ident("vcpkg-boost-helper"));
+    CHECK(ID::is_ident("lpt"));
+    CHECK(ID::is_ident("com"));
 
     // reject invalid characters
     CHECK(!ID::is_ident(""));
+    CHECK(!ID::is_ident(" "));
     CHECK(!ID::is_ident("boost_tuple"));
     CHECK(!ID::is_ident("boost.tuple"));
+    CHECK(!ID::is_ident("boost."));
     CHECK(!ID::is_ident("boost@1"));
     CHECK(!ID::is_ident("boost#1"));
     CHECK(!ID::is_ident("boost:x64-windows"));
@@ -140,8 +144,10 @@ TEST_CASE ("check valid package patterns", "[registries]")
     CHECK(!ID::is_ident("con"));
     CHECK(!ID::is_ident("core"));
     CHECK(!ID::is_ident("default"));
-    CHECK(!ID::is_ident("lpt1"));
+    CHECK(!ID::is_ident("lpt0"));
-    CHECK(!ID::is_ident("com1"));
+    CHECK(!ID::is_ident("lpt9"));
+    CHECK(!ID::is_ident("com0"));
+    CHECK(!ID::is_ident("com9"));
 
     // reject incomplete segments
     CHECK(!ID::is_ident("-a"));
@@ -154,11 +160,17 @@ TEST_CASE ("check valid package patterns", "[registries]")
     CHECK(is_package_pattern("b*"));
     CHECK(is_package_pattern("boost*"));
     CHECK(is_package_pattern("boost-*"));
+    CHECK(is_package_pattern("boost-multi-*"));
 
     // reject invalid patterns
+    CHECK(!is_package_pattern(""));
+    CHECK(!is_package_pattern(" "));
     CHECK(!is_package_pattern("*a"));
     CHECK(!is_package_pattern("a*a"));
     CHECK(!is_package_pattern("a**"));
+    CHECK(!is_package_pattern("a-**"));
+    CHECK(!is_package_pattern("a--*"));
+    CHECK(!is_package_pattern("a-*-*"));
     CHECK(!is_package_pattern("a+"));
     CHECK(!is_package_pattern("a?"));
 }

src/vcpkg/base/json.cpp

@@ -1080,12 +1080,13 @@ namespace vcpkg::Json
 
         if (sv.size() < 5)
         {
+            // see https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions
             if (sv == "prn" || sv == "aux" || sv == "nul" || sv == "con" || sv == FeatureNameCore)
             {
                 return false; // we're a reserved identifier
             }
             if (sv.size() == 4 && (Strings::starts_with(sv, "lpt") || Strings::starts_with(sv, "com")) &&
-                sv[3] >= '1' && sv[3] <= '9')
+                sv[3] >= '0' && sv[3] <= '9')
             {
                 return false; // we're a reserved identifier
             }

src/vcpkg/configuration.cpp

@@ -1022,32 +1022,34 @@ namespace vcpkg
             return true;
         }
 
-        /*if (sv == "*")
+        // ([a-z0-9]+-)*([a-z0-9]+[*]?|[*])
-        {
-            return true;
-        }*/
-
-        // ([a-z0-9]+(-[a-z0-9]+)*)(\*?)
         auto cur = sv.begin();
         const auto last = sv.end();
+        // each iteration of this loop matches either
+        // ([a-z0-9]+-)
+        // or the last
+        // ([a-z0-9]+[*]?|[*])
         for (;;)
         {
-            // [a-z0-9]+
             if (cur == last)
             {
                 return false;
             }
 
+            // this if checks for the first matched character of [a-z0-9]+
             if (!ParserBase::is_lower_digit(*cur))
             {
-                if (*cur != '*')
+                // [a-z0-9]+ didn't match anything, so we must be matching
+                // the last [*]
+                if (*cur == '*')
                 {
-                    return false;
+                    return ++cur == last;
                 }
 
-                return ++cur == last;
+                return false;
             }
 
+            // match the rest of the [a-z0-9]+
             do
             {
                 ++cur;
@@ -1060,11 +1062,11 @@ namespace vcpkg
             switch (*cur)
             {
                 case '-':
-                    // repeat outer [a-z0-9]+ again to match -[a-z0-9]+
+                    // this loop iteration matched [a-z0-9]+- once
                     ++cur;
                     continue;
                 case '*':
-                    // match last optional *
+                    // this loop matched [a-z0-9]+[*]? (and [*] was present)
                     ++cur;
                     return cur == last;
                 default: return false;