Skip to content

Comments

feat: Introduce the templated_uri crate family#265

Open
kate-shine wants to merge 8 commits intomainfrom
u/kchuranov/uri
Open

feat: Introduce the templated_uri crate family#265
kate-shine wants to merge 8 commits intomainfrom
u/kchuranov/uri

Conversation

@kate-shine
Copy link

No description provided.

@github-actions
Copy link

github-actions bot commented Feb 16, 2026

⚠️ Breaking Changes Detected

error: failed to retrieve local crate data from git revision

Caused by:
    0: failed to retrieve manifest file from git revision source
    1: possibly due to errors: [
         failed when reading /home/runner/work/oxidizer/oxidizer/target/semver-checks/git-origin_main/cfdd6907fa17efa8e70de9bf37a1298d6b445f32/scripts/crate-template/Cargo.toml: TOML parse error at line 9, column 26
         |
       9 | keywords = ["oxidizer", {{CRATE_KEYWORDS}}]
         |                          ^
       missing key for inline table element, expected key
       : TOML parse error at line 9, column 26
         |
       9 | keywords = ["oxidizer", {{CRATE_KEYWORDS}}]
         |                          ^
       missing key for inline table element, expected key
       ,
         failed to parse /home/runner/work/oxidizer/oxidizer/target/semver-checks/git-origin_main/cfdd6907fa17efa8e70de9bf37a1298d6b445f32/Cargo.toml: no `package` table,
       ]
    2: package `templated_uri` not found in /home/runner/work/oxidizer/oxidizer/target/semver-checks/git-origin_main/cfdd6907fa17efa8e70de9bf37a1298d6b445f32

Stack backtrace:
   0: anyhow::error::<impl anyhow::Error>::msg
   1: cargo_semver_checks::rustdoc_gen::RustdocFromProjectRoot::get_crate_source
   2: cargo_semver_checks::rustdoc_gen::StatefulRustdocGenerator<cargo_semver_checks::rustdoc_gen::CoupledState>::prepare_generator
   3: cargo_semver_checks::Check::check_release::{{closure}}
   4: cargo_semver_checks::Check::check_release
   5: cargo_semver_checks::exit_on_error
   6: cargo_semver_checks::main
   7: std::sys::backtrace::__rust_begin_short_backtrace
   8: main
   9: <unknown>
  10: __libc_start_main
  11: _start

If the breaking changes are intentional then everything is fine - this message is merely informative.

Remember to apply a version number bump with the correct severity when publishing a version with breaking changes (1.x.x -> 2.x.x or 0.1.x -> 0.2.x).

@codecov
Copy link

codecov bot commented Feb 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (bc20e71) to head (73e0750).

Additional details and impacted files
@@            Coverage Diff            @@
##             main     #265     +/-   ##
=========================================
  Coverage   100.0%   100.0%             
=========================================
  Files         141      156     +15     
  Lines        8640    10801   +2161     
=========================================
+ Hits         8640    10801   +2161     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kate-shine kate-shine force-pushed the u/kchuranov/uri branch 16 times, most recently from 00578d3 to c21aef9 Compare February 20, 2026 12:55
@kate-shine kate-shine force-pushed the u/kchuranov/uri branch 6 times, most recently from 530fefd to 4dde26f Compare February 20, 2026 14:46
@kate-shine kate-shine force-pushed the u/kchuranov/uri branch 4 times, most recently from fe5df3f to 73be0ad Compare February 20, 2026 16:04
@geeknoid
Copy link
Member

Note that the logo I designed for when the crate was named "obscuri". The new name warrants a new logo.

Also, you should update the PR title.

@geeknoid geeknoid requested a review from Copilot February 20, 2026 18:40
@geeknoid
Copy link
Member

Here are a bunch of things the AI recommends fixing:

Analysis of templated_uri Crate Family

Review of all source files across the three crates (templated_uri, templated_uri_macros, templated_uri_macros_impl).

🐛 Correctness Issues

1. UriSafeString::new() rejects percent-encoded values (RFC 6570 compliance gap)

File: crates/templated_uri/src/uri_safe.rs:120

The reserved-character check rejects characters like : and /, but per RFC 6570 §1.5, the output of Simple String Expansion should be percent-encoded. Currently, UriSafeString values are interpolated verbatim via format!() with no percent-encoding step. If a UriSafeString contains a space or other unreserved-but-special character (e.g. ~, which is allowed through validation), it will be placed into the URI without encoding, potentially producing an invalid URI. The crate's blocklist also omits % itself — a string like "foo%ZZbar" (invalid percent-encoding) would pass validation and produce a malformed URI.

2. TryFrom<String> for UriSafeString always copies the string

File: crates/templated_uri/src/uri_safe.rs:213-214

Self::new(&s) calls s.to_string() again inside new(), double-allocating. The owned String is discarded and a fresh clone is created via ToString. The Cow::Owned should reuse the already-owned String.

3. RedactedDisplay for struct templates misses prefix separators in param groups

File: crates/templated_uri_macros_impl/src/struct_template.rs:224-259

The construct_redacted_display function emits Content parts (static template segments) correctly, but for ParamGroup it emits each parameter inline without the group's prefix/separator. For example, a {?q1,q2} group would emit q1={value}&q2={value} in to_uri_string (via format_template), but in RedactedDisplay, each param is emitted without the ?, &, or key= parts. This means the redacted output for query parameters and semicolon-KV parameters will have structurally different output than the non-redacted version — missing ?, &, ; prefixes and key= labels.

4. Typo in Cargo.toml keywords

File: crates/templated_uri/Cargo.toml:9

"psrsing" should be "parsing".

5. Origin::with_port has a typo in expect message

File: crates/templated_uri/src/base_uri/origin.rs:112

"Scheme ahd host are already valid""ahd" should be "and".

6. Unclosed doc-comment block on with_origin

File: crates/templated_uri/src/base_uri.rs:420

The #[must_use] attribute follows directly after the example code block without a closing /// doc-comment line, which means the attribute is technically outside the doc block.

⚡ Performance Issues

7. UriSafeString::new() takes &impl ToString instead of accepting owned strings

File: crates/templated_uri/src/uri_safe.rs:112

The signature pub fn new(s: &impl ToString) always calls .to_string(), which allocates even when the caller already has a String. A more efficient API would accept impl Into<String> or have separate new/from_str paths. The TryFrom<String> impl at line 213 passes &s to new(), which calls s.to_string() again — a needless copy.

8. Uri::to_string() allocates without capacity hint

File: crates/templated_uri/src/uri.rs:119-130

to_string() builds a String by first calling ToString::to_string() on BaseUri (which goes through Display::fmt), then appending the path. For frequently called code paths, pre-allocating with String::with_capacity would reduce reallocations.

9. TargetPathAndQuery::template() clones then converts PathAndQuery

File: crates/templated_uri/src/uri.rs:297

Cow::Owned(classified_pq.clone().declassify_ref().to_string()) clones the entire Sensitive<PathAndQuery> just to produce a string. It could use classified_pq.declassify_ref().to_string() directly without cloning.

10. UriSafeString validation uses str::contains per character

File: crates/templated_uri/src/uri_safe.rs:120

For each character in the input, "{}/:?#[]@!$&'()*+,;=".contains(c) does a linear scan through 20 characters. A lookup table or matches!() on byte values (like from_static already uses) would be faster for long strings.

🧩 Usability Issues

11. Uri::to_string() shadows std::fmt::Display

File: crates/templated_uri/src/uri.rs:119

Uri has an inherent to_string() method returning Sensitive<String>, which shadows the usual Display::to_string(). This is intentional for data privacy, but Uri doesn't implement Display at all, which can be surprising. The method name could cause confusion — consider to_sensitive_string() or similar.

12. UriSafe is a sealed trait with no prominent documentation of that fact

File: crates/templated_uri/src/uri_safe.rs:41-58

Users can't implement UriSafe for their own types (sealed trait). For custom newtypes that wrap safe types, users must use UriFragment/UriUnsafeFragment derive macros. The sealed nature is intentional but the docs could mention this more prominently.

13. Missing Eq/PartialEq on Uri

File: crates/templated_uri/src/uri.rs:50-56

Uri doesn't implement PartialEq or Eq. Tests at lines 479-481 compare via .to_string() instead. Since both BaseUri and TargetPathAndQuery (via string comparison) can be compared, adding at least string-based equality would be convenient.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new crate family for standards-compliant URI handling with templating, safety validation, and data classification support. However, there is a significant naming discrepancy: the PR title and description reference "obscuri" while the actual crate names are "templated_uri", "templated_uri_macros", and "templated_uri_macros_impl". The implementation provides RFC 6570 Level 3 compliant URI templating with compile-time safety guarantees and integration with the data_privacy crate for redaction support.

Changes:

  • Adds three new crates: templated_uri, templated_uri_macros, and templated_uri_macros_impl
  • Implements RFC 6570 URI templates with derive macros for type-safe URI construction
  • Provides UriSafe trait and validation to prevent URI injection vulnerabilities

Reviewed changes

Copilot reviewed 39 out of 40 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
crates/templated_uri/src/lib.rs Main library module with core types and re-exports
crates/templated_uri/src/uri.rs Uri type implementation with templating support
crates/templated_uri/src/uri_safe.rs UriSafe trait and validation logic
crates/templated_uri/src/uri_fragment.rs Fragment traits for URI template parameters
crates/templated_uri/src/base_uri.rs BaseUri type for scheme and authority handling
crates/templated_uri_macros/src/lib.rs Procedural macro entry points
crates/templated_uri_macros_impl/src/lib.rs Core macro implementation logic
crates/templated_uri_macros_impl/src/template_parser.rs RFC 6570 template parser using chumsky
crates/templated_uri_macros_impl/src/struct_template.rs Struct template macro implementation
crates/templated_uri_macros_impl/src/enum_template.rs Enum template macro implementation
crates/templated_uri_macros_impl/src/uri_fragment.rs Derive macro for UriFragment traits
Cargo.toml Workspace dependency additions
README.md Updated with new crate reference (naming issue)
CHANGELOG.md Updated with new crate references (naming issues)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +6 to +7
#![doc(html_logo_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/CRATE_NAME/logo.png")]
#![doc(html_favicon_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/CRATE_NAME/favicon.ico")]
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The html_logo_url and html_favicon_url contain "CRATE_NAME" placeholder text that should be replaced with "templated_uri" to properly reference the assets for this specific crate.

Suggested change
#![doc(html_logo_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/CRATE_NAME/logo.png")]
#![doc(html_favicon_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/CRATE_NAME/favicon.ico")]
#![doc(html_logo_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/templated_uri/logo.png")]
#![doc(html_favicon_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/templated_uri/favicon.ico")]

Copilot uses AI. Check for mistakes.
#[expect(clippy::missing_panics_doc, reason = "impossible panic")]
pub fn with_port(self, port: u16) -> Self {
let host = self.authority.host();
Self::new(self.scheme, format!("{host}:{port}")).expect("Scheme ahd host are already valid and port is a valid u16")
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

Typo in expect message: "ahd" should be "and". The message should read "Scheme and host are already valid..."

Suggested change
Self::new(self.scheme, format!("{host}:{port}")).expect("Scheme ahd host are already valid and port is a valid u16")
Self::new(self.scheme, format!("{host}:{port}")).expect("Scheme and host are already valid and port is a valid u16")

Copilot uses AI. Check for mistakes.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#![expect(clippy::option_if_let_else, reason = "Darling's macro exapansion currently uses this pattern")]
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

Typo in comment: "exapansion" should be "expansion".

Copilot uses AI. Check for mistakes.
impl templated_uri::TemplatedPathAndQuery for #ident {
fn rfc_6570_template(&self) -> &'static core::primitive::str {
match self {
#(#variant_matches =>template_variant.rfc_6570_template()),*
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

Missing space after "=>". Should be => template_variant.rfc_6570_template() to match Rust formatting conventions.

Copilot uses AI. Check for mistakes.
- [`data_privacy`](./crates/data_privacy/README.md) - Mechanisms to classify, manipulate, and redact sensitive data.
- [`fundle`](./crates/fundle/README.md) - Compile-time safe dependency injection for Rust.
- [`layered`](./crates/layered/README.md) - A foundational service abstraction for building composable, middleware-driven systems.
- [`obscuri`](./crates/obscuri/README.md) - Standards-compliant URI handling with templating, safety validation, and data classification
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The PR title and description mention "obscuri" but the actual crate name is "templated_uri". This entry should read "templated_uri" instead of "obscuri" to match the actual crate name used throughout the codebase.

Copilot uses AI. Check for mistakes.
description = "Standards-compliant URI handling with templating, safety validation, and data classification"
version = "0.1.0"
readme = "README.md"
keywords = ["oxidizer", "url", "psrsing", "templates", "uri"]
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

Typo in keyword: "psrsing" should be "parsing".

Suggested change
keywords = ["oxidizer", "url", "psrsing", "templates", "uri"]
keywords = ["oxidizer", "url", "parsing", "templates", "uri"]

Copilot uses AI. Check for mistakes.
Comment on lines +7 to +8
#![doc(html_logo_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/CRATE_NAME/logo.png")]
#![doc(html_favicon_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/CRATE_NAME/favicon.ico")]
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The html_logo_url and html_favicon_url contain "CRATE_NAME" placeholder text that should be replaced with "templated_uri_macros" to properly reference the assets for this specific crate.

Suggested change
#![doc(html_logo_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/CRATE_NAME/logo.png")]
#![doc(html_favicon_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/CRATE_NAME/favicon.ico")]
#![doc(html_logo_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/templated_uri_macros/logo.png")]
#![doc(html_favicon_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/templated_uri_macros/favicon.ico")]

Copilot uses AI. Check for mistakes.
@geeknoid geeknoid changed the title feat: Introduce the obscuri crate family feat: Introduce the templated_uri crate family Feb 20, 2026
repository = "https://github.com/microsoft/oxidizer/tree/main/crates/templated_uri"

[package.metadata.cargo_check_external_types]
allowed_external_types = [
Copy link
Member

Choose a reason for hiding this comment

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

I worry that we're having to expose all of these external types here. Are we OK coupling this crate to the http crate?

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.

2 participants