[Fix #1166] Relax URI patterns to conform to RFC 3986#1169
[Fix #1166] Relax URI patterns to conform to RFC 3986#1169ricardozanini wants to merge 7 commits into
Conversation
…Uri pattern Replace the absolute-only URI pattern with the RFC 3986 Appendix B URI-reference regex, which accepts both absolute and relative URIs. This allows values like `openapi/petstore.json` or `proto/greeter.proto`. LiteralUriTemplate keeps the absolute pattern since templates need a scheme. Signed-off-by: Ricardo Zanini <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR updates the JSON Schema definition for LiteralUri in schema/workflow.yaml to accept relative URI references (e.g., openapi/petstore.json) in addition to absolute URIs with schemes, aligning with RFC 3986 usage in practice.
Changes:
- Replaced the
LiteralUriabsolute-only URI regex with an RFC 3986 URI-reference–style regex to allow relative URIs. - Kept
LiteralUriTemplateas absolute-only to preserve existinguri-templatevalidation constraints.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Update LiteralUriTemplate pattern to RFC 3986 Appendix B regex, matching the LiteralUri change - Add reasoning in dsl-reference.md that URI types conform to RFC 3986 URI-reference syntax - Add 39 tests for URI pattern validation Signed-off-by: Ricardo Zanini <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
schema/workflow.yaml:1445
- The RFC 3986 Appendix B regex is a parsing regex and, as used here, it matches the empty string and also matches whitespace-containing/non-URI strings (because the path component
[^?#]*can consume nearly anything). This significantly relaxes validation compared to the previous scheme-required pattern, especially in validators that don’t enforceformat. Consider at least requiring non-empty values and excluding whitespace/control characters so obvious invalid inputs don’t validate.
- title: LiteralUriTemplate
type: string
format: uri-template
pattern: "^(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\\?([^#]*))?(#(.*))?"
.ci/validation/src/uri-pattern.test.ts:63
- Only positive cases are asserted. Adding a small set of negative cases (e.g., empty string and whitespace-containing strings) would prevent regressions where the regex becomes effectively “match anything.”
const networkPathUris = ["//example.com/path", "//localhost:8080/api"];
test.each(absoluteUris)("accepts absolute URI: %s", (uri) => {
expect(URI_REFERENCE_PATTERN.test(uri)).toBe(true);
});
The RFC 3986 Appendix B regex matches any string (including empty
and runtime expressions), breaking oneOf disambiguation. Add end
anchor, negative lookahead for ${ expressions, and require at
least one non-whitespace character.
Signed-off-by: Ricardo Zanini <[email protected]>
- Change LiteralUri format from uri to uri-reference to semantically match relative reference support - Fix dsl-reference.md link to RFC 3986 §4.1 (URI-reference definition) instead of Appendix B (parsing regex) Signed-off-by: Ricardo Zanini <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
schema/workflow.yaml:1445
- The new URI-reference regex still accepts whitespace inside the value (e.g., trailing spaces or
http://example.com/has space) because([^?#]*)allows spaces and the only guard is(?=\S)(which only checks the first character). That contradicts RFC 3986 URI-reference syntax (spaces must be percent-encoded) and also undermines the intent of using a pattern for structural validation whenformatvalidation is inconsistent.
Consider strengthening the pattern to reject any whitespace and (optionally) restore the stricter scheme grammar (while keeping the scheme optional so relative references remain valid).
pattern: "^(?!\\s*\\$\\{)(?=\\S)(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\\?([^#]*))?(#(.*))?$"
schema/workflow.yaml:1449
- Same concern as above: the current pattern allows whitespace anywhere after the first character and doesn’t enforce RFC 3986’s no-whitespace requirement. Tightening the regex here too keeps
LiteralUrialigned with the stated RFC 3986 URI-reference constraint while still allowing relative references.
pattern: "^(?!\\s*\\$\\{)(?=\\S)(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\\?([^#]*))?(#(.*))?$"
dsl-reference.md:2472
- This sentence states that URI/URI-template types “conform” to RFC 3986 URI-reference syntax, but the schema is using a regex derived from Appendix B (a parsing regex) and it’s intentionally more permissive than the full RFC grammar. To avoid over-claiming strict conformance, it would be clearer to describe this as structural validation based on RFC 3986’s URI-reference regex while noting that it accepts both absolute and relative references.
All URI and URI template types in the DSL conform to the [URI-reference](https://datatracker.ietf.org/doc/html/rfc3986#section-4.1) syntax defined by [RFC 3986](https://datatracker.ietf.org/doc/html/rfc3986), meaning both absolute URIs (e.g., `https://example.com/path`) and relative references (e.g., `openapi/petstore.json`, `/api/v1/users`) are accepted.
.ci/validation/src/uri-pattern.test.ts:139
- Given the schema change is intended to follow RFC 3986 URI-reference syntax, it’s worth adding explicit negative cases for whitespace inside an otherwise-valid URI (e.g., trailing spaces or spaces in the path). This prevents regressions where the pattern accidentally accepts unencoded spaces.
test("rejects empty string", () => {
expect(URI_REFERENCE_PATTERN.test("")).toBe(false);
});
Read LiteralUri and LiteralUriTemplate patterns directly from schema/workflow.yaml instead of duplicating the regex in a JS literal. Also assert both patterns are identical. Signed-off-by: Ricardo Zanini <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
.ci/validation/src/uri-pattern.test.ts:22
- The tests currently hardcode a regex literal, so they can drift from the actual schema pattern in schema/workflow.yaml without failing. Loading the pattern from the schema (and asserting both LiteralUri and LiteralUriTemplate use the same pattern) makes these tests validate the real contract.
import * as fs from "fs";
import * as path from "path";
import * as yaml from "js-yaml";
const schemaPath = path.resolve(__dirname, "../../../schema/workflow.yaml");
const schema = yaml.load(
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
schema/workflow.yaml:1449
- The RFC 3986 Appendix B regex is a parsing regex and as written here it will still accept whitespace inside the reference (e.g.,
openapi/pet store.json,https://example.com/has space), which is not valid RFC 3986 URI-reference syntax. If the intent is true RFC 3986 conformance, add a no-whitespace guard to the pattern.
pattern: "^(?!\\s*\\$\\{)(?=\\S)(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\\?([^#]*))?(#(.*))?$"
- title: LiteralUri
type: string
format: uri-reference
pattern: "^(?!\\s*\\$\\{)(?=\\S)(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\\?([^#]*))?(#(.*))?$"
.ci/validation/src/uri-pattern.test.ts:145
- The new URI-reference pattern currently has no negative test coverage for embedded whitespace (which the Appendix B parsing regex would otherwise match). Adding explicit rejection tests for whitespace-containing references will prevent regressions if the pattern is tightened to better match RFC 3986.
describe("URI pattern rejects invalid or ambiguous values", () => {
const runtimeExpressions = [
"${ .foo }",
" ${ .bar } ",
"${.baz}",
- Use node:fs and node:path import specifiers to match project convention - Clarify that URI templates are validated before variable expansion, avoiding implying strict RFC 3986 conformance for unexpanded templates Signed-off-by: Ricardo Zanini <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
.ci/validation/src/uri-pattern.test.ts:162
- Given the intent to conform to RFC 3986 URI-reference syntax, it would be good to explicitly test that the pattern rejects URIs containing unencoded whitespace (spaces/newlines). The current negative tests cover empty/whitespace-only strings but not embedded whitespace.
test("rejects whitespace-only string", () => {
expect(LITERAL_URI_PATTERN.test(" ")).toBe(false);
});
| pattern: "^(?!\\s*\\$\\{)(?=\\S)(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\\?([^#]*))?(#(.*))?$" | ||
| - title: LiteralUri | ||
| type: string | ||
| format: uri | ||
| pattern: "^[A-Za-z][A-Za-z0-9+\\-.]*://.*" | ||
| format: uri-reference | ||
| pattern: "^(?!\\s*\\$\\{)(?=\\S)(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\\?([^#]*))?(#(.*))?$" |
There was a problem hiding this comment.
The pattern's primary purpose is disambiguating uriTemplate from runtimeExpression in oneOf branches — not full URI syntax validation. Strict character-level validation (rejecting unencoded whitespace, invalid percent-encoding, etc.) is delegated to the format: uri / format: uri-reference validators, which JSON Schema implementations handle at evaluation time.
Tightening the regex to exclude whitespace mid-string (e.g., [^?#\s]*) risks rejecting edge cases and adds complexity for a concern the format keyword already covers. The (?=\S) lookahead prevents empty/whitespace-only strings, which was the practical issue.
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Summary
LiteralUriandLiteralUriTemplatepatterns only accepted absolute URIs with a schemeopenapi/petstore.jsonorproto/greeter.protoare valid per RFC 3986 and used in practiceContext
The original absolute-only pattern was added in #1017 to work around
format: uri-templateinconsistencies across JSON Schema validators. The RFC 3986 regex maintains structural validation while supporting relative references.Changes
schema/workflow.yaml: Updated bothLiteralUriandLiteralUriTemplatepatterns to RFC 3986 Appendix B regexdsl-reference.md: Added reasoning that all URI types conform to RFC 3986 URI-reference syntax.ci/validation/src/uri-pattern.test.ts: Added 39 tests covering absolute URIs, relative paths, query/fragment combinations, network-path references, and URI templatesFixes #1166