Fix router keyword handling (fix #442)router-keywords-fix

4 files changed, 161 insertions, 149 deletions

diff --git a/src/api/admin/router.rs b/src/api/admin/router.rs
index 3fa07b3c..62e6abc3 100644
--- a/src/api/admin/router.rs
+++ b/src/api/admin/router.rs
@@ -143,10 +143,13 @@ impl Endpoint {
 }
 
 generateQueryParameters! {
-	"format" => format,
-	"id" => id,
-	"search" => search,
-	"globalAlias" => global_alias,
-	"alias" => alias,
-	"accessKeyId" => access_key_id
+	keywords: [],
+	fields: [
+		"format" => format,
+		"id" => id,
+		"search" => search,
+		"globalAlias" => global_alias,
+		"alias" => alias,
+		"accessKeyId" => access_key_id
+	]
 }
diff --git a/src/api/k2v/router.rs b/src/api/k2v/router.rs
index 50e6965b..e7a3dd69 100644
--- a/src/api/k2v/router.rs
+++ b/src/api/k2v/router.rs
@@ -96,7 +96,7 @@ impl Endpoint {
 	fn from_get(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
 		router_match! {
 			@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: [
 				EMPTY if causality_token => PollItem (query::sort_key, query::causality_token, opt_parse::timeout),
 				EMPTY => ReadItem (query::sort_key),
@@ -111,7 +111,7 @@ impl Endpoint {
 	fn from_search(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
 		router_match! {
 			@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: [
 			],
 			no_key: [
@@ -125,7 +125,7 @@ impl Endpoint {
 	fn from_head(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
 		router_match! {
 			@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: [
 				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> {
 		router_match! {
 			@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: [
 			],
 			no_key: [
@@ -155,7 +155,7 @@ impl Endpoint {
 	fn from_put(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
 		router_match! {
 			@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: [
 				EMPTY => InsertItem (query::sort_key),
 
@@ -169,7 +169,7 @@ impl Endpoint {
 	fn from_delete(partition_key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
 		router_match! {
 			@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: [
 				EMPTY => DeleteItem (query::sort_key),
 			],
@@ -232,21 +232,18 @@ impl Endpoint {
 
 // parameter name => struct field
 generateQueryParameters! {
-	"prefix" => prefix,
-	"start" => start,
-	"causality_token" => causality_token,
-	"end" => end,
-	"limit" => limit,
-	"reverse" => reverse,
-	"sort_key" => sort_key,
-	"timeout" => timeout
-}
-
-mod keywords {
-	//! This module contain all query parameters with no associated value
-	//! used to differentiate endpoints.
-	pub const EMPTY: &str = "";
-
-	pub const DELETE: &str = "delete";
-	pub const SEARCH: &str = "search";
+	keywords: [
+		"delete" => DELETE,
+		"search" => SEARCH
+	],
+	fields: [
+		"prefix" => prefix,
+		"start" => start,
+		"causality_token" => causality_token,
+		"end" => end,
+		"limit" => limit,
+		"reverse" => reverse,
+		"sort_key" => sort_key,
+		"timeout" => timeout
+	]
 }
diff --git a/src/api/router_macros.rs b/src/api/router_macros.rs
index 4c593300..959e69a3 100644
--- a/src/api/router_macros.rs
+++ b/src/api/router_macros.rs
@@ -4,10 +4,9 @@ macro_rules! router_match {
     (@match $enum:expr , [ $($endpoint:ident,)* ]) => {{
         // usage: router_match {@match my_enum, [ VariantWithField1, VariantWithField2 ..] }
         // returns true if the variant was one of the listed variants, false otherwise.
-        use Endpoint::*;
         match $enum {
             $(
-            $endpoint { .. } => true,
+            Endpoint::$endpoint { .. } => true,
             )*
             _ => false
         }
@@ -15,37 +14,35 @@ macro_rules! router_match {
     (@extract $enum:expr , $param:ident, [ $($endpoint:ident,)* ]) => {{
         // 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.
-        use Endpoint::*;
         match $enum {
             $(
-            $endpoint {$param, ..} => Some($param),
+            Endpoint::$endpoint {$param, ..} => Some($param),
             )*
             _ => None
         }
     }};
-	(@gen_path_parser ($method:expr, $reqpath:expr, $query:expr)
-	 [
-	 $($meth:ident $path:pat $(if $required:ident)? => $api:ident $(($($conv:ident :: $param:ident),*))?,)*
-	 ]) => {{
-		{
-			use Endpoint::*;
-			match ($method, $reqpath) {
-				$(
-					(&Method::$meth, $path) if true $(&& $query.$required.is_some())? => $api {
-						$($(
-							$param: router_match!(@@parse_param $query, $conv, $param),
-						)*)?
-					},
-				)*
-				(m, p) => {
-					return Err(Error::bad_request(format!(
-						"Unknown API endpoint: {} {}",
-						m, p
-					)))
-				}
-			}
-		}
-	}};
+    (@gen_path_parser ($method:expr, $reqpath:expr, $query:expr)
+     [
+     $($meth:ident $path:pat $(if $required:ident)? => $api:ident $(($($conv:ident :: $param:ident),*))?,)*
+     ]) => {{
+        {
+            match ($method, $reqpath) {
+                $(
+                    (&Method::$meth, $path) if true $(&& $query.$required.is_some())? => Endpoint::$api {
+                        $($(
+                            $param: router_match!(@@parse_param $query, $conv, $param),
+                        )*)?
+                    },
+                )*
+                (m, p) => {
+                    return Err(Error::bad_request(format!(
+                        "Unknown API endpoint: {} {}",
+                        m, p
+                    )))
+                }
+            }
+        }
+    }};
     (@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),*))?,)*],
         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.
-        use Endpoint::*;
-        use keywords::*;
         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,
                 $($(
                     $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),
                 )*)?
@@ -84,7 +79,7 @@ macro_rules! router_match {
 
     (@@parse_param $query:expr, query_opt, $param:ident) => {{
         // 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) => {{
         // extract mendatory query parameter
@@ -93,7 +88,7 @@ macro_rules! router_match {
     (@@parse_param $query:expr, opt_parse, $param:ident) => {{
         // extract and parse optional query parameter
         // missing parameter is file, however parse error is reported as an error
-		$query.$param
+        $query.$param
             .take()
             .map(|param| param.parse())
             .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
 /// is useless outside of this module.
 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,
         /// but with keys statically known.
         #[derive(Debug, Default)]
         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> {
                 let mut res: Self = Default::default();
                 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() {
-                                res.$name.replace(v).is_some()
-                            } else {
-                                false
+                            $kw_param => if let Some(prev_kw) = res.keyword.replace(Keyword::$kw_name) {
+                                return Err(Error::bad_request(format!(
+                                    "Multiple keywords: '{}' and '{}'", prev_kw, $kw_param
+                                )));
                             },
                         )*
-                        _ => {
-                            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"));
+                        $(
+                            $f_param => if !v.is_empty() {
+                                if res.$f_name.replace(v).is_some() {
+                                    return Err(Error::bad_request(format!(
+                                        "Query parameter repeated: '{}'", k
+                                    )));
                                 }
-                                continue;
-                            } else {
+                            },
+                        )*
+                        _ => {
+                            if !(k.starts_with("response-") || k.starts_with("X-Amz-")) {
                                 debug!("Received an unknown query parameter: '{}'", k);
-                                false
                             }
                         }
                     };
-                    if repeated {
-                        return Err(Error::bad_request(format!(
-                            "Query parameter repeated: '{}'",
-                            k
-                        )));
-                    }
                 }
                 Ok(res)
             }
@@ -198,8 +213,8 @@ macro_rules! generateQueryParameters {
                 if self.keyword.is_some() {
                     Some("Keyword not used")
                 } $(
-                    else if self.$name.is_some() {
-                        Some(concat!("'", $rest, "'"))
+                    else if self.$f_name.is_some() {
+                        Some(concat!("'", $f_param, "'"))
                     }
                 )* else {
                     None
diff --git a/src/api/s3/router.rs b/src/api/s3/router.rs
index 44f581ff..821b0e07 100644
--- a/src/api/s3/router.rs
+++ b/src/api/s3/router.rs
@@ -355,7 +355,7 @@ impl Endpoint {
 	fn from_get(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
 		router_match! {
 			@gen_parser
-			(query.keyword.take().unwrap_or_default().as_ref(), key, query, None),
+			(query.keyword.take().unwrap_or_default(), key, query, None),
 			key: [
 				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),
@@ -412,7 +412,7 @@ impl Endpoint {
 	fn from_head(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
 		router_match! {
 			@gen_parser
-			(query.keyword.take().unwrap_or_default().as_ref(), key, query, None),
+			(query.keyword.take().unwrap_or_default(), key, query, None),
 			key: [
 				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> {
 		router_match! {
 			@gen_parser
-			(query.keyword.take().unwrap_or_default().as_ref(), key, query, None),
+			(query.keyword.take().unwrap_or_default(), key, query, None),
 			key: [
 				EMPTY if upload_id  => CompleteMultipartUpload (query::upload_id),
 				RESTORE => RestoreObject (query_opt::version_id),
@@ -448,7 +448,7 @@ impl Endpoint {
 	) -> Result<Self, Error> {
 		router_match! {
 			@gen_parser
-			(query.keyword.take().unwrap_or_default().as_ref(), key, query, headers),
+			(query.keyword.take().unwrap_or_default(), key, query, headers),
 			key: [
 				EMPTY if part_number header "x-amz-copy-source" => UploadPartCopy (parse::part_number, query::upload_id),
 				EMPTY header "x-amz-copy-source" => CopyObject,
@@ -490,7 +490,7 @@ impl Endpoint {
 	fn from_delete(key: String, query: &mut QueryParameters<'_>) -> Result<Self, Error> {
 		router_match! {
 			@gen_parser
-			(query.keyword.take().unwrap_or_default().as_ref(), key, query, None),
+			(query.keyword.take().unwrap_or_default(), key, query, None),
 			key: [
 				EMPTY if upload_id => AbortMultipartUpload (query::upload_id),
 				EMPTY => DeleteObject (query_opt::version_id),
@@ -624,63 +624,60 @@ impl Endpoint {
 
 // parameter name => struct field
 generateQueryParameters! {
-	"continuation-token" => continuation_token,
-	"delimiter" => delimiter,
-	"encoding-type" => encoding_type,
-	"fetch-owner" => fetch_owner,
-	"id" => id,
-	"key-marker" => key_marker,
-	"list-type" => list_type,
-	"marker" => marker,
-	"max-keys" => max_keys,
-	"max-parts" => max_parts,
-	"max-uploads" => max_uploads,
-	"partNumber" => part_number,
-	"part-number-marker" => part_number_marker,
-	"prefix" => prefix,
-	"select-type" => select_type,
-	"start-after" => start_after,
-	"uploadId" => upload_id,
-	"upload-id-marker" => upload_id_marker,
-	"versionId" => version_id,
-	"version-id-marker" => version_id_marker
-}
-
-mod keywords {
-	//! This module contain all query parameters with no associated value S3 uses to differentiate
-	//! endpoints.
-	pub const EMPTY: &str = "";
-
-	pub const ACCELERATE: &str = "accelerate";
-	pub const ACL: &str = "acl";
-	pub const ANALYTICS: &str = "analytics";
-	pub const CORS: &str = "cors";
-	pub const DELETE: &str = "delete";
-	pub const ENCRYPTION: &str = "encryption";
-	pub const INTELLIGENT_TIERING: &str = "intelligent-tiering";
-	pub const INVENTORY: &str = "inventory";
-	pub const LEGAL_HOLD: &str = "legal-hold";
-	pub const LIFECYCLE: &str = "lifecycle";
-	pub const LOCATION: &str = "location";
-	pub const LOGGING: &str = "logging";
-	pub const METRICS: &str = "metrics";
-	pub const NOTIFICATION: &str = "notification";
-	pub const OBJECT_LOCK: &str = "object-lock";
-	pub const OWNERSHIP_CONTROLS: &str = "ownershipControls";
-	pub const POLICY: &str = "policy";
-	pub const POLICY_STATUS: &str = "policyStatus";
-	pub const PUBLIC_ACCESS_BLOCK: &str = "publicAccessBlock";
-	pub const REPLICATION: &str = "replication";
-	pub const REQUEST_PAYMENT: &str = "requestPayment";
-	pub const RESTORE: &str = "restore";
-	pub const RETENTION: &str = "retention";
-	pub const SELECT: &str = "select";
-	pub const TAGGING: &str = "tagging";
-	pub const TORRENT: &str = "torrent";
-	pub const UPLOADS: &str = "uploads";
-	pub const VERSIONING: &str = "versioning";
-	pub const VERSIONS: &str = "versions";
-	pub const WEBSITE: &str = "website";
+	keywords: [
+		"accelerate" => ACCELERATE,
+		"acl" => ACL,
+		"analytics" => ANALYTICS,
+		"cors" => CORS,
+		"delete" => DELETE,
+		"encryption" => ENCRYPTION,
+		"intelligent-tiering" => INTELLIGENT_TIERING,
+		"inventory" => INVENTORY,
+		"legal-hold" => LEGAL_HOLD,
+		"lifecycle" => LIFECYCLE,
+		"location" => LOCATION,
+		"logging" => LOGGING,
+		"metrics" => METRICS,
+		"notification" => NOTIFICATION,
+		"object-lock" => OBJECT_LOCK,
+		"ownershipControls" => OWNERSHIP_CONTROLS,
+		"policy" => POLICY,
+		"policyStatus" => POLICY_STATUS,
+		"publicAccessBlock" => PUBLIC_ACCESS_BLOCK,
+		"replication" => REPLICATION,
+		"requestPayment" => REQUEST_PAYMENT,
+		"restore" => RESTORE,
+		"retention" => RETENTION,
+		"select" => SELECT,
+		"tagging" => TAGGING,
+		"torrent" => TORRENT,
+		"uploads" => UPLOADS,
+		"versioning" => VERSIONING,
+		"versions" => VERSIONS,
+		"website" => WEBSITE
+	],
+	fields: [
+		"continuation-token" => continuation_token,
+		"delimiter" => delimiter,
+		"encoding-type" => encoding_type,
+		"fetch-owner" => fetch_owner,
+		"id" => id,
+		"key-marker" => key_marker,
+		"list-type" => list_type,
+		"marker" => marker,
+		"max-keys" => max_keys,
+		"max-parts" => max_parts,
+		"max-uploads" => max_uploads,
+		"partNumber" => part_number,
+		"part-number-marker" => part_number_marker,
+		"prefix" => prefix,
+		"select-type" => select_type,
+		"start-after" => start_after,
+		"uploadId" => upload_id,
+		"upload-id-marker" => upload_id_marker,
+		"versionId" => version_id,
+		"version-id-marker" => version_id_marker
+	]
 }
 
 #[cfg(test)]