feat(builtins): add uri.is_valid and uri.parse - #80
Open
anakrish wants to merge 3 commits into
Open
Conversation
microsoft#794) Bumps the per-dependency group in /bindings/ruby with 1 update: [rb_sys](https://github.com/oxidize-rb/rb-sys). Updates `rb_sys` from 0.9.128 to 0.9.130 - [Release notes](https://github.com/oxidize-rb/rb-sys/releases) - [Commits](oxidize-rb/rb-sys@v0.9.128...v0.9.130) --- updated-dependencies: - dependency-name: rb_sys dependency-version: 0.9.130 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: per-dependency ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Both builtins require the 'urlquery' feature (which provides the url crate).
uri.is_valid(uri) - returns true for syntactically valid, non-empty URIs.
uri.parse(uri) - parses a URI and returns an object with keys: scheme,
hostname, port (if explicit), path / raw_path (for
hierarchical URIs with a path component), raw_query,
fragment. Empty fields are omitted. Passes empty string
back as an empty object. Percent-decodes the path into the
'path' key while preserving the original in 'raw_path'.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds uri.is_valid and uri.parse builtins (behind the urlquery feature) to improve OPA v1.19.1 compatibility, with new interpreter YAML test coverage and a docs entry.
Changes:
- Register new
uri.is_validanduri.parsebuiltins insrc/builtins/encoding.rs - Implement URI parsing/validation helpers (explicit port detection, explicit path detection, percent-decoding)
- Add YAML interpreter tests and list the new builtins in documentation
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/interpreter/cases/builtins/encoding/uri.yaml | Adds interpreter test cases for uri.is_valid and uri.parse. |
| src/builtins/encoding.rs | Implements and registers uri.is_valid/uri.parse and supporting helpers. |
| docs/builtins.md | Adds the new builtins to the builtin-to-feature documentation table. |
| bindings/ruby/Gemfile.lock | Bumps rb_sys dependency version in the Ruby binding lockfile. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+91
to
+111
| insert("scheme", parsed.scheme()); | ||
| let hostname = parsed.host_str().unwrap_or_default(); | ||
| insert( | ||
| "hostname", | ||
| hostname | ||
| .strip_prefix('[') | ||
| .and_then(|hostname| hostname.strip_suffix(']')) | ||
| .unwrap_or(hostname), | ||
| ); | ||
| if let Some(port) = explicit_uri_port(&uri) { | ||
| insert("port", port); | ||
| } | ||
| if !parsed.cannot_be_a_base() && has_explicit_uri_path(&uri) { | ||
| let raw_path = parsed.path(); | ||
| let path = percent_decode(raw_path) | ||
| .map_err(|err| params[0].span().error(&format!("invalid URI path: {err}")))?; | ||
| insert("path", &path); | ||
| insert("raw_path", raw_path); | ||
| } | ||
| insert("raw_query", parsed.query().unwrap_or_default()); | ||
| insert("fragment", parsed.fragment().unwrap_or_default()); |
Comment on lines
+82
to
+83
| let parsed = | ||
| url::Url::parse(&uri).map_err(|err| params[0].span().error(&err.to_string()))?; |
Comment on lines
+55
to
+61
| _strict: bool, | ||
| ) -> Result<Value> { | ||
| let name = "uri.is_valid"; | ||
| ensure_args_count(span, name, params, args, 1)?; | ||
| let uri = match ensure_string(name, ¶ms[0], &args[0]) { | ||
| Ok(uri) => uri, | ||
| Err(_) => return Ok(Value::Bool(false)), |
Comment on lines
+90
to
+98
| - note: is_valid-non-string-undefined | ||
| strict: false | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = uri.is_valid(42) | ||
| query: data.test.x | ||
| want_result: false |
| package test | ||
| x = uri.parse("://missing-scheme") | ||
| query: data.test.x | ||
| error: "relative URL without a base" |
Comment on lines
+152
to
+171
| fn percent_decode(value: &str) -> Result<String> { | ||
| let bytes = value.as_bytes(); | ||
| let mut decoded = Vec::with_capacity(bytes.len()); | ||
| let mut idx = 0; | ||
| while idx < bytes.len() { | ||
| if bytes[idx] == b'%' { | ||
| let high = bytes | ||
| .get(idx.saturating_add(1)) | ||
| .and_then(|byte| char::from(*byte).to_digit(16)); | ||
| let low = bytes | ||
| .get(idx.saturating_add(2)) | ||
| .and_then(|byte| char::from(*byte).to_digit(16)); | ||
| let (Some(high), Some(low)) = (high, low) else { | ||
| bail!("invalid percent encoding"); | ||
| }; | ||
| decoded.push(u8::try_from(high * 16 + low)?); | ||
| idx = idx.saturating_add(3); | ||
| } else { | ||
| decoded.push(bytes[idx]); | ||
| idx = idx.saturating_add(1); |
- uri.is_valid strict flag: parameter was _strict (unused). Now properly propagates an error in strict mode when a non-string argument is given, and returns false in non-strict mode (matching OPA behavior). - uri.parse error message stability: change from bare url-crate message to a stable prefix 'invalid uri: ...' so tests can match on a stable substring rather than a brittle crate-internal string. - Rename test note: is_valid-non-string-undefined -> is_valid-non-string-false (the function returns false, not undefined, for non-string args). - Add tests: - is_valid-non-string-strict-error: strict=true + non-string arg errors - parse-ipv6-with-port: http://[::1]:8080/path (IPv6 brackets stripped in hostname) - parse-empty-path: https://example.com?q=1 (no path/raw_path when absent) - parse-invalid-percent-encoding: path with incomplete %xx sequence errors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
uri.is_validanduri.parsebuiltins for OPA v1.19.1 compatibility.Changes
uri.is_valid(uri)— returnstrueif the URI string is syntactically validuri.parse(uri)— parses a URI string into an object with scheme, userinfo, host, port, path, query, fragment componentsdocs/builtins.md— document new builtinsBoth functions are added as inner functions inside
register()inencoding.rs, consistent with the existing URI-related helpers in that module.Tests
New YAML test file:
tests/interpreter/cases/builtins/encoding/uri.yaml(9 test cases).Part of OPA v1.2.0 → v1.19.1 upgrade.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com