aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlex Auvolat <alex@adnab.me>2022-12-12 11:56:40 +0100
committerAlex Auvolat <alex@adnab.me>2022-12-12 12:05:37 +0100
commita0abf417626be8d120f660c582195747d131b88b (patch)
tree585aa99a5fb41511460580b4cf7df7fa851fc5e4
parent980572a8872c56ea9572ff03579ebb9a65013775 (diff)
downloadgarage-router-keywords-fix.tar.gz
garage-router-keywords-fix.zip
Fix router keyword handling (fix #442)router-keywords-fix
-rw-r--r--src/api/admin/router.rs15
-rw-r--r--src/api/k2v/router.rs43
-rw-r--r--src/api/router_macros.rs131
-rw-r--r--src/api/s3/router.rs121
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)]