Merge pull request 'Fix router keyword handling (fix #442)' (#446) from router-keywords-fix into main

Reviewed-on: Deuxfleurs/garage#446
This commit is contained in:
Alex 2022-12-15 08:40:26 +00:00
commit 1af4a5ed56
4 changed files with 161 additions and 149 deletions

View file

@ -143,10 +143,13 @@ impl Endpoint {
} }
generateQueryParameters! { generateQueryParameters! {
"format" => format, keywords: [],
"id" => id, fields: [
"search" => search, "format" => format,
"globalAlias" => global_alias, "id" => id,
"alias" => alias, "search" => search,
"accessKeyId" => access_key_id "globalAlias" => global_alias,
"alias" => alias,
"accessKeyId" => access_key_id
]
} }

View file

@ -96,7 +96,7 @@ impl Endpoint {
fn from_get(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_get(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), partition_key, query, None), (query.keyword.take().unwrap_or_default(), partition_key, query, None),
key: [ key: [
EMPTY if causality_token => PollItem (query::sort_key, query::causality_token, opt_parse::timeout), EMPTY if causality_token => PollItem (query::sort_key, query::causality_token, opt_parse::timeout),
EMPTY => ReadItem (query::sort_key), EMPTY => ReadItem (query::sort_key),
@ -111,7 +111,7 @@ impl Endpoint {
fn from_search(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_search(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), partition_key, query, None), (query.keyword.take().unwrap_or_default(), partition_key, query, None),
key: [ key: [
], ],
no_key: [ no_key: [
@ -125,7 +125,7 @@ impl Endpoint {
fn from_head(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_head(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), partition_key, query, None), (query.keyword.take().unwrap_or_default(), partition_key, query, None),
key: [ key: [
EMPTY => HeadObject(opt_parse::part_number, query_opt::version_id), EMPTY => HeadObject(opt_parse::part_number, query_opt::version_id),
], ],
@ -140,7 +140,7 @@ impl Endpoint {
fn from_post(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_post(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), partition_key, query, None), (query.keyword.take().unwrap_or_default(), partition_key, query, None),
key: [ key: [
], ],
no_key: [ no_key: [
@ -155,7 +155,7 @@ impl Endpoint {
fn from_put(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_put(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), partition_key, query, None), (query.keyword.take().unwrap_or_default(), partition_key, query, None),
key: [ key: [
EMPTY => InsertItem (query::sort_key), EMPTY => InsertItem (query::sort_key),
@ -169,7 +169,7 @@ impl Endpoint {
fn from_delete(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_delete(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), partition_key, query, None), (query.keyword.take().unwrap_or_default(), partition_key, query, None),
key: [ key: [
EMPTY => DeleteItem (query::sort_key), EMPTY => DeleteItem (query::sort_key),
], ],
@ -232,21 +232,18 @@ impl Endpoint {
// parameter name => struct field // parameter name => struct field
generateQueryParameters! { generateQueryParameters! {
"prefix" => prefix, keywords: [
"start" => start, "delete" => DELETE,
"causality_token" => causality_token, "search" => SEARCH
"end" => end, ],
"limit" => limit, fields: [
"reverse" => reverse, "prefix" => prefix,
"sort_key" => sort_key, "start" => start,
"timeout" => timeout "causality_token" => causality_token,
} "end" => end,
"limit" => limit,
mod keywords { "reverse" => reverse,
//! This module contain all query parameters with no associated value "sort_key" => sort_key,
//! used to differentiate endpoints. "timeout" => timeout
pub const EMPTY: &str = ""; ]
pub const DELETE: &str = "delete";
pub const SEARCH: &str = "search";
} }

View file

@ -4,10 +4,9 @@ macro_rules! router_match {
(@match $enum:expr , [ $($endpoint:ident,)* ]) => {{ (@match $enum:expr , [ $($endpoint:ident,)* ]) => {{
// usage: router_match {@match my_enum, [ VariantWithField1, VariantWithField2 ..] } // usage: router_match {@match my_enum, [ VariantWithField1, VariantWithField2 ..] }
// returns true if the variant was one of the listed variants, false otherwise. // returns true if the variant was one of the listed variants, false otherwise.
use Endpoint::*;
match $enum { match $enum {
$( $(
$endpoint { .. } => true, Endpoint::$endpoint { .. } => true,
)* )*
_ => false _ => false
} }
@ -15,37 +14,35 @@ macro_rules! router_match {
(@extract $enum:expr , $param:ident, [ $($endpoint:ident,)* ]) => {{ (@extract $enum:expr , $param:ident, [ $($endpoint:ident,)* ]) => {{
// usage: router_match {@extract my_enum, field_name, [ VariantWithField1, VariantWithField2 ..] } // usage: router_match {@extract my_enum, field_name, [ VariantWithField1, VariantWithField2 ..] }
// returns Some(field_value), or None if the variant was not one of the listed variants. // returns Some(field_value), or None if the variant was not one of the listed variants.
use Endpoint::*;
match $enum { match $enum {
$( $(
$endpoint {$param, ..} => Some($param), Endpoint::$endpoint {$param, ..} => Some($param),
)* )*
_ => None _ => None
} }
}}; }};
(@gen_path_parser ($method:expr, $reqpath:expr, $query:expr) (@gen_path_parser ($method:expr, $reqpath:expr, $query:expr)
[ [
$($meth:ident $path:pat $(if $required:ident)? => $api:ident $(($($conv:ident :: $param:ident),*))?,)* $($meth:ident $path:pat $(if $required:ident)? => $api:ident $(($($conv:ident :: $param:ident),*))?,)*
]) => {{ ]) => {{
{ {
use Endpoint::*; match ($method, $reqpath) {
match ($method, $reqpath) { $(
$( (&Method::$meth, $path) if true $(&& $query.$required.is_some())? => Endpoint::$api {
(&Method::$meth, $path) if true $(&& $query.$required.is_some())? => $api { $($(
$($( $param: router_match!(@@parse_param $query, $conv, $param),
$param: router_match!(@@parse_param $query, $conv, $param), )*)?
)*)? },
}, )*
)* (m, p) => {
(m, p) => { return Err(Error::bad_request(format!(
return Err(Error::bad_request(format!( "Unknown API endpoint: {} {}",
"Unknown API endpoint: {} {}", m, p
m, p )))
))) }
} }
} }
} }};
}};
(@gen_parser ($keyword:expr, $key:ident, $query:expr, $header:expr), (@gen_parser ($keyword:expr, $key:ident, $query:expr, $header:expr),
key: [$($kw_k:ident $(if $required_k:ident)? $(header $header_k:expr)? => $api_k:ident $(($($conv_k:ident :: $param_k:ident),*))?,)*], key: [$($kw_k:ident $(if $required_k:ident)? $(header $header_k:expr)? => $api_k:ident $(($($conv_k:ident :: $param_k:ident),*))?,)*],
no_key: [$($kw_nk:ident $(if $required_nk:ident)? $(if_header $header_nk:expr)? => $api_nk:ident $(($($conv_nk:ident :: $param_nk:ident),*))?,)*]) => {{ no_key: [$($kw_nk:ident $(if $required_nk:ident)? $(if_header $header_nk:expr)? => $api_nk:ident $(($($conv_nk:ident :: $param_nk:ident),*))?,)*]) => {{
@ -60,11 +57,9 @@ macro_rules! router_match {
// ] // ]
// } // }
// See in from_{method} for more detailed usage. // See in from_{method} for more detailed usage.
use Endpoint::*;
use keywords::*;
match ($keyword, !$key.is_empty()){ match ($keyword, !$key.is_empty()){
$( $(
($kw_k, true) if true $(&& $query.$required_k.is_some())? $(&& $header.contains_key($header_k))? => Ok($api_k { (Keyword::$kw_k, true) if true $(&& $query.$required_k.is_some())? $(&& $header.contains_key($header_k))? => Ok(Endpoint::$api_k {
$key, $key,
$($( $($(
$param_k: router_match!(@@parse_param $query, $conv_k, $param_k), $param_k: router_match!(@@parse_param $query, $conv_k, $param_k),
@ -72,7 +67,7 @@ macro_rules! router_match {
}), }),
)* )*
$( $(
($kw_nk, false) $(if $query.$required_nk.is_some())? $(if $header.contains($header_nk))? => Ok($api_nk { (Keyword::$kw_nk, false) $(if $query.$required_nk.is_some())? $(if $header.contains($header_nk))? => Ok(Endpoint::$api_nk {
$($( $($(
$param_nk: router_match!(@@parse_param $query, $conv_nk, $param_nk), $param_nk: router_match!(@@parse_param $query, $conv_nk, $param_nk),
)*)? )*)?
@ -84,7 +79,7 @@ macro_rules! router_match {
(@@parse_param $query:expr, query_opt, $param:ident) => {{ (@@parse_param $query:expr, query_opt, $param:ident) => {{
// extract optional query parameter // extract optional query parameter
$query.$param.take().map(|param| param.into_owned()) $query.$param.take().map(|param| param.into_owned())
}}; }};
(@@parse_param $query:expr, query, $param:ident) => {{ (@@parse_param $query:expr, query, $param:ident) => {{
// extract mendatory query parameter // extract mendatory query parameter
@ -93,7 +88,7 @@ macro_rules! router_match {
(@@parse_param $query:expr, opt_parse, $param:ident) => {{ (@@parse_param $query:expr, opt_parse, $param:ident) => {{
// extract and parse optional query parameter // extract and parse optional query parameter
// missing parameter is file, however parse error is reported as an error // missing parameter is file, however parse error is reported as an error
$query.$param $query.$param
.take() .take()
.map(|param| param.parse()) .map(|param| param.parse())
.transpose() .transpose()
@ -144,14 +139,39 @@ macro_rules! router_match {
/// This macro is used to generate part of the code in this module. It must be called only one, and /// This macro is used to generate part of the code in this module. It must be called only one, and
/// is useless outside of this module. /// is useless outside of this module.
macro_rules! generateQueryParameters { macro_rules! generateQueryParameters {
( $($rest:expr => $name:ident),* ) => { (
keywords: [ $($kw_param:expr => $kw_name: ident),* ],
fields: [ $($f_param:expr => $f_name:ident),* ]
) => {
#[derive(Debug)]
#[allow(non_camel_case_types)]
enum Keyword {
EMPTY,
$( $kw_name, )*
}
impl std::fmt::Display for Keyword {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Keyword::EMPTY => write!(f, "``"),
$( Keyword::$kw_name => write!(f, "`{}`", $kw_param), )*
}
}
}
impl Default for Keyword {
fn default() -> Self {
Keyword::EMPTY
}
}
/// Struct containing all query parameters used in endpoints. Think of it as an HashMap, /// Struct containing all query parameters used in endpoints. Think of it as an HashMap,
/// but with keys statically known. /// but with keys statically known.
#[derive(Debug, Default)] #[derive(Debug, Default)]
struct QueryParameters<'a> { struct QueryParameters<'a> {
keyword: Option<Cow<'a, str>>, keyword: Option<Keyword>,
$( $(
$name: Option<Cow<'a, str>>, $f_name: Option<Cow<'a, str>>,
)* )*
} }
@ -160,34 +180,29 @@ macro_rules! generateQueryParameters {
fn from_query(query: &'a str) -> Result<Self, Error> { fn from_query(query: &'a str) -> Result<Self, Error> {
let mut res: Self = Default::default(); let mut res: Self = Default::default();
for (k, v) in url::form_urlencoded::parse(query.as_bytes()) { for (k, v) in url::form_urlencoded::parse(query.as_bytes()) {
let repeated = match k.as_ref() { match k.as_ref() {
$( $(
$rest => if !v.is_empty() { $kw_param => if let Some(prev_kw) = res.keyword.replace(Keyword::$kw_name) {
res.$name.replace(v).is_some() return Err(Error::bad_request(format!(
} else { "Multiple keywords: '{}' and '{}'", prev_kw, $kw_param
false )));
},
)*
$(
$f_param => if !v.is_empty() {
if res.$f_name.replace(v).is_some() {
return Err(Error::bad_request(format!(
"Query parameter repeated: '{}'", k
)));
}
}, },
)* )*
_ => { _ => {
if k.starts_with("response-") || k.starts_with("X-Amz-") { if !(k.starts_with("response-") || k.starts_with("X-Amz-")) {
false
} else if v.as_ref().is_empty() {
if res.keyword.replace(k).is_some() {
return Err(Error::bad_request("Multiple keywords"));
}
continue;
} else {
debug!("Received an unknown query parameter: '{}'", k); debug!("Received an unknown query parameter: '{}'", k);
false
} }
} }
}; };
if repeated {
return Err(Error::bad_request(format!(
"Query parameter repeated: '{}'",
k
)));
}
} }
Ok(res) Ok(res)
} }
@ -198,8 +213,8 @@ macro_rules! generateQueryParameters {
if self.keyword.is_some() { if self.keyword.is_some() {
Some("Keyword not used") Some("Keyword not used")
} $( } $(
else if self.$name.is_some() { else if self.$f_name.is_some() {
Some(concat!("'", $rest, "'")) Some(concat!("'", $f_param, "'"))
} }
)* else { )* else {
None None

View file

@ -355,7 +355,7 @@ impl Endpoint {
fn from_get(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_get(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), key, query, None), (query.keyword.take().unwrap_or_default(), key, query, None),
key: [ key: [
EMPTY if upload_id => ListParts (query::upload_id, opt_parse::max_parts, opt_parse::part_number_marker), EMPTY if upload_id => ListParts (query::upload_id, opt_parse::max_parts, opt_parse::part_number_marker),
EMPTY => GetObject (query_opt::version_id, opt_parse::part_number), EMPTY => GetObject (query_opt::version_id, opt_parse::part_number),
@ -412,7 +412,7 @@ impl Endpoint {
fn from_head(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_head(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), key, query, None), (query.keyword.take().unwrap_or_default(), key, query, None),
key: [ key: [
EMPTY => HeadObject(opt_parse::part_number, query_opt::version_id), EMPTY => HeadObject(opt_parse::part_number, query_opt::version_id),
], ],
@ -426,7 +426,7 @@ impl Endpoint {
fn from_post(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_post(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), key, query, None), (query.keyword.take().unwrap_or_default(), key, query, None),
key: [ key: [
EMPTY if upload_id => CompleteMultipartUpload (query::upload_id), EMPTY if upload_id => CompleteMultipartUpload (query::upload_id),
RESTORE => RestoreObject (query_opt::version_id), RESTORE => RestoreObject (query_opt::version_id),
@ -448,7 +448,7 @@ impl Endpoint {
) -> Result<Self, Error> { ) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), key, query, headers), (query.keyword.take().unwrap_or_default(), key, query, headers),
key: [ key: [
EMPTY if part_number header "x-amz-copy-source" => UploadPartCopy (parse::part_number, query::upload_id), EMPTY if part_number header "x-amz-copy-source" => UploadPartCopy (parse::part_number, query::upload_id),
EMPTY header "x-amz-copy-source" => CopyObject, EMPTY header "x-amz-copy-source" => CopyObject,
@ -490,7 +490,7 @@ impl Endpoint {
fn from_delete(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> { fn from_delete(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
router_match! { router_match! {
@gen_parser @gen_parser
(query.keyword.take().unwrap_or_default().as_ref(), key, query, None), (query.keyword.take().unwrap_or_default(), key, query, None),
key: [ key: [
EMPTY if upload_id => AbortMultipartUpload (query::upload_id), EMPTY if upload_id => AbortMultipartUpload (query::upload_id),
EMPTY => DeleteObject (query_opt::version_id), EMPTY => DeleteObject (query_opt::version_id),
@ -624,63 +624,60 @@ impl Endpoint {
// parameter name => struct field // parameter name => struct field
generateQueryParameters! { generateQueryParameters! {
"continuation-token" => continuation_token, keywords: [
"delimiter" => delimiter, "accelerate" => ACCELERATE,
"encoding-type" => encoding_type, "acl" => ACL,
"fetch-owner" => fetch_owner, "analytics" => ANALYTICS,
"id" => id, "cors" => CORS,
"key-marker" => key_marker, "delete" => DELETE,
"list-type" => list_type, "encryption" => ENCRYPTION,
"marker" => marker, "intelligent-tiering" => INTELLIGENT_TIERING,
"max-keys" => max_keys, "inventory" => INVENTORY,
"max-parts" => max_parts, "legal-hold" => LEGAL_HOLD,
"max-uploads" => max_uploads, "lifecycle" => LIFECYCLE,
"partNumber" => part_number, "location" => LOCATION,
"part-number-marker" => part_number_marker, "logging" => LOGGING,
"prefix" => prefix, "metrics" => METRICS,
"select-type" => select_type, "notification" => NOTIFICATION,
"start-after" => start_after, "object-lock" => OBJECT_LOCK,
"uploadId" => upload_id, "ownershipControls" => OWNERSHIP_CONTROLS,
"upload-id-marker" => upload_id_marker, "policy" => POLICY,
"versionId" => version_id, "policyStatus" => POLICY_STATUS,
"version-id-marker" => version_id_marker "publicAccessBlock" => PUBLIC_ACCESS_BLOCK,
} "replication" => REPLICATION,
"requestPayment" => REQUEST_PAYMENT,
mod keywords { "restore" => RESTORE,
//! This module contain all query parameters with no associated value S3 uses to differentiate "retention" => RETENTION,
//! endpoints. "select" => SELECT,
pub const EMPTY: &str = ""; "tagging" => TAGGING,
"torrent" => TORRENT,
pub const ACCELERATE: &str = "accelerate"; "uploads" => UPLOADS,
pub const ACL: &str = "acl"; "versioning" => VERSIONING,
pub const ANALYTICS: &str = "analytics"; "versions" => VERSIONS,
pub const CORS: &str = "cors"; "website" => WEBSITE
pub const DELETE: &str = "delete"; ],
pub const ENCRYPTION: &str = "encryption"; fields: [
pub const INTELLIGENT_TIERING: &str = "intelligent-tiering"; "continuation-token" => continuation_token,
pub const INVENTORY: &str = "inventory"; "delimiter" => delimiter,
pub const LEGAL_HOLD: &str = "legal-hold"; "encoding-type" => encoding_type,
pub const LIFECYCLE: &str = "lifecycle"; "fetch-owner" => fetch_owner,
pub const LOCATION: &str = "location"; "id" => id,
pub const LOGGING: &str = "logging"; "key-marker" => key_marker,
pub const METRICS: &str = "metrics"; "list-type" => list_type,
pub const NOTIFICATION: &str = "notification"; "marker" => marker,
pub const OBJECT_LOCK: &str = "object-lock"; "max-keys" => max_keys,
pub const OWNERSHIP_CONTROLS: &str = "ownershipControls"; "max-parts" => max_parts,
pub const POLICY: &str = "policy"; "max-uploads" => max_uploads,
pub const POLICY_STATUS: &str = "policyStatus"; "partNumber" => part_number,
pub const PUBLIC_ACCESS_BLOCK: &str = "publicAccessBlock"; "part-number-marker" => part_number_marker,
pub const REPLICATION: &str = "replication"; "prefix" => prefix,
pub const REQUEST_PAYMENT: &str = "requestPayment"; "select-type" => select_type,
pub const RESTORE: &str = "restore"; "start-after" => start_after,
pub const RETENTION: &str = "retention"; "uploadId" => upload_id,
pub const SELECT: &str = "select"; "upload-id-marker" => upload_id_marker,
pub const TAGGING: &str = "tagging"; "versionId" => version_id,
pub const TORRENT: &str = "torrent"; "version-id-marker" => version_id_marker
pub const UPLOADS: &str = "uploads"; ]
pub const VERSIONING: &str = "versioning";
pub const VERSIONS: &str = "versions";
pub const WEBSITE: &str = "website";
} }
#[cfg(test)] #[cfg(test)]