Skip to content

Commit 0183b0f

Browse files
authored
soft-disallow prefix resources with tail segments (#379)
1 parent b122a1a commit 0183b0f

File tree

1 file changed

+23
-5
lines changed

1 file changed

+23
-5
lines changed

actix-router/src/resource.rs

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,13 +1070,32 @@ impl ResourceDef {
10701070
dyn_segment_count += 1;
10711071
}
10721072

1073+
if is_prefix && has_tail_segment {
1074+
// tail segments in prefixes have no defined semantics
1075+
1076+
#[cfg(not(test))]
1077+
log::warn!(
1078+
"Prefix resources should not have tail segments. \
1079+
Use `ResourceDef::new` constructor. \
1080+
This may become a panic in the future."
1081+
);
1082+
1083+
// panic in tests to make this case detectable
1084+
#[cfg(test)]
1085+
panic!("prefix resource definitions should not have tail segments");
1086+
}
1087+
10731088
if unprocessed.ends_with('*') {
10741089
// unnamed tail segment
10751090

10761091
#[cfg(not(test))]
1077-
log::warn!("tail segments must have names; consider `{{tail}}*`");
1092+
log::warn!(
1093+
"Tail segments must have names. \
1094+
Consider `.../{{tail}}*`. \
1095+
This may become a panic in the future."
1096+
);
10781097

1079-
// to this case detectable in tests
1098+
// panic in tests to make this case detectable
10801099
#[cfg(test)]
10811100
panic!("tail segments must have names");
10821101
} else if !has_tail_segment && !unprocessed.is_empty() {
@@ -1197,7 +1216,6 @@ mod tests {
11971216

11981217
assert_eq!(ResourceDef::new("/"), ResourceDef::new(["/"]));
11991218
assert_eq!(ResourceDef::new("/"), ResourceDef::new(vec!["/"]));
1200-
assert_eq!(ResourceDef::new("/{id}*"), ResourceDef::prefix("/{id}*"));
12011219

12021220
assert_ne!(ResourceDef::new(""), ResourceDef::prefix(""));
12031221
assert_ne!(ResourceDef::new("/"), ResourceDef::prefix("/"));
@@ -1774,11 +1792,11 @@ mod tests {
17741792
#[test]
17751793
#[should_panic]
17761794
fn invalid_unnamed_tail_segment() {
1777-
ResourceDef::new(r"/*");
1795+
ResourceDef::new("/*");
17781796
}
17791797

17801798
#[test]
1781-
// #[should_panic] // TODO: consider if this should be allowed
1799+
#[should_panic]
17821800
fn prefix_plus_tail_match_is_allowed() {
17831801
ResourceDef::prefix("/user/{id}*");
17841802
}

0 commit comments

Comments
 (0)