fix(transcode): emit axum 0.8 path syntax in proto_path_to_axum#18
Merged
Conversation
The 1.0.2 axum 0.7 -> 0.8 bump left proto_path_to_axum converting
proto `{id}` templates to the old `:id` capture syntax. axum 0.8
rejects any path segment starting with `:` at Router::route(), so
every brace-param http rule panicked the REST transcoding router at
startup ("Path segments must not start with `:`"). gRPC was fine; the
whole REST listener was dead.
axum 0.8's capture syntax IS the google.api.http `{param}` form, so
plain params now pass through verbatim. Field-path and wildcard
templates are rewritten correctly:
- `{name=*}` -> `{name}`
- `{name=**}` -> `{*name}` (catch-all)
- bare `*` / `**` -> position-named captures
Flipped the unit test to assert 0.8 output and added a regression test
that builds the real router over a brace-param path (the panic site)
and asserts it does not panic.
Closes #17
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesAxum 0.8 path conversion fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~23 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
| Filename | Overview |
|---|---|
| src/transcode/mod.rs | Replaces the single-pass {…}→:param rewrite with a brace-depth-aware segment splitter and a proper axum 0.8 path emitter; adds four regression tests including live router construction. |
Reviews (3): Last reviewed commit: "fix(transcode): degrade non-terminal cat..." | Re-trigger Greptile
google.api.http resource-name templates embed slashes inside a single
brace span, e.g. `{name=shelves/*/books/*}`. proto_path_to_axum splits
on `/` before inspecting brace captures, fracturing such a span into
invalid fragments and emitting a mangled axum path that panics at
Router::route(). This test reproduces the failure; the fix follows.
Part of #17
proto_path_to_axum split the path on `/` before inspecting brace
captures, so a google.api.http multi-segment field template like
`{name=shelves/*/books/*}` (slashes embedded in one capture) fractured
into invalid fragments and emitted a mangled axum path that panicked at
Router::route().
Split only at top-level `/` (tracking brace depth) so each capture
stays intact. Multi-segment field templates with interspersed literals
have no faithful axum representation, so collapse them to a `{*name}`
catch-all and emit a tracing::warn, surfacing the limitation instead of
silently mis-routing. Single-segment `{name=*}` / `{name=**}` and plain
`{name}` captures are unchanged.
Part of #17
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/transcode/mod.rs (1)
468-473:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNon-terminal unsupported field templates can still panic router registration.
On Line 472 / Lines 521-528, unsupported
{name=...}templates always become{*name}. If that segment is not last (e.g./v1/{name=projects/*}/topics), the output becomes/v1/{*name}/topics, which axum rejects atRouter::route().Proposed fix
pub fn proto_path_to_axum(path: &str) -> String { let mut out = String::with_capacity(path.len()); + let segments = split_top_level(path); - for (idx, segment) in split_top_level(path).into_iter().enumerate() { + for (idx, segment) in segments.iter().enumerate() { if idx > 0 { out.push('/'); } - out.push_str(&convert_segment(segment, idx)); + let is_last = idx + 1 == segments.len(); + out.push_str(&convert_segment(segment, idx, is_last)); } out } @@ -fn convert_segment(segment: &str, idx: usize) -> String { +fn convert_segment(segment: &str, idx: usize, is_last: bool) -> String { @@ _ => { tracing::warn!( template = %inner, "google.api.http multi-segment field template is not fully \ supported; routing it as a catch-all capture" ); - format!("{{*{name}}}") + if is_last { + format!("{{*{name}}}") + } else { + tracing::warn!( + template = %inner, + "non-terminal multi-segment template cannot be represented as axum catch-all; degrading to single-segment capture" + ); + format!("{{{name}}}") + } } }; }+ #[test] + fn non_terminal_multi_segment_template_does_not_generate_mid_catch_all() { + assert_eq!( + proto_path_to_axum("/v1/{name=projects/*}/topics"), + "/v1/{name}/topics" + ); + let path = proto_path_to_axum("/v1/{name=projects/*}/topics"); + let _router: Router<()> = Router::new().route(&path, get(|| async { "ok" })); + }Also applies to: 507-529
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/transcode/mod.rs` around lines 468 - 473, The convert_segment function converts all unsupported field templates like {name=projects/*} to {*name}, but catch-all patterns ({*name}) are only valid in the last path segment. In the loop iterating through split_top_level results, pass information about whether the current segment is terminal (the last one) to convert_segment by either passing a boolean parameter indicating if this is the final segment or by passing the total segment count. Then modify the convert_segment function to check if an unsupported template is being processed in a non-terminal position and either return an error or handle it differently instead of unconditionally converting to the {*name} catch-all pattern which will cause axum Router::route() to reject the route registration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/transcode/mod.rs`:
- Around line 468-473: The convert_segment function converts all unsupported
field templates like {name=projects/*} to {*name}, but catch-all patterns
({*name}) are only valid in the last path segment. In the loop iterating through
split_top_level results, pass information about whether the current segment is
terminal (the last one) to convert_segment by either passing a boolean parameter
indicating if this is the final segment or by passing the total segment count.
Then modify the convert_segment function to check if an unsupported template is
being processed in a non-terminal position and either return an error or handle
it differently instead of unconditionally converting to the {*name} catch-all
pattern which will cause axum Router::route() to reject the route registration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 24080bee-3587-45ef-8b04-e399d3b9c08c
📒 Files selected for processing (1)
src/transcode/mod.rs
axum permits a catch-all capture `{*name}` only in the LAST path
segment. convert_segment collapses unsupported / `**` field templates
to `{*name}` unconditionally, so a non-terminal template like
`/v1/{name=projects/*}/topics` produces `/v1/{*name}/topics`, which
axum rejects at Router::route(). This test reproduces it; fix follows.
Part of #17
convert_segment collapsed unsupported / `**` field templates to an axum
catch-all `{*name}` regardless of position. axum accepts a catch-all
only in the final path segment, so a non-terminal template such as
`/v1/{name=projects/*}/topics` produced `/v1/{*name}/topics` and
panicked the router at Router::route().
Thread `is_last` through convert_segment and route every catch-all
emission through a `catch_all` helper that produces `{*name}` only for
the terminal segment, otherwise degrades to a single-segment `{name}`
capture and warns. Single-segment and plain captures are unchanged.
Part of #17
Contributor
|
🎉 This PR is included in version 1.0.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This was referenced Jun 19, 2026
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
1 participant
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
The 1.0.2 axum 0.7 → 0.8 bump left
proto_path_to_axumemitting the old:paramcapture syntax. axum 0.8 rejects any path segment starting with:atRouter::route(), so everygoogle.api.httprule with a{param}template panicked the REST transcoding router at startup. gRPC was unaffected; the whole REST listener was dead.Fix
axum 0.8's capture syntax matches the
google.api.http{param}form, so plain params now pass through verbatim. Field-path / wildcard templates are rewritten:{name}{name}{name=*}{name}{name=**}{*name}(catch-all)*/**Testing
test_proto_path_to_axumto assert 0.8 output.test_proto_path_to_axum_wildcardscovering field-path/catch-all/bare wildcards.router_builds_with_brace_path_params_on_axum_0_8— builds the real router over the brace-param path (the panic site) and asserts no panic. Pre-fix this panicked.cargo nextest run: 35 passed.cargo clippy --all-targets: clean.Release
semantic-release will cut 1.0.3 from this
fix:commit. 1.0.2 is not yanked (it carries the jsonwebtoken 10.4.0 / GHSA-h395 security fix); 1.0.3 keeps that and repairs transcoding.Closes #17