Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace bool with enum for a function parameter of label::fmt_string #1875

Merged
merged 8 commits into from
Oct 25, 2022
Merged
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,9 @@ message = "Support Sigv4 signature generation on PowerPC 32 and 64 bit. This arc
references = ["smithy-rs#1847"]
meta = { "breaking" = true, "tada" = false, "bug" = true }
author = "crisidev"

[[smithy-rs]]
message = "Replace bool with enum for a function parameter of `label::fmt_string`."
references = ["smithy-rs#1875"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }
author = "ysaito1001"
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ pub(super) fn percent_encode_query(value: &str) -> String {
}

pub(super) fn percent_encode_path(value: &str) -> String {
label::fmt_string(value, true)
label::fmt_string(value, label::EncodingStrategy::Greedy)
}
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,12 @@ class RequestBindingGenerator(
when {
target.isStringShape -> {
val func = format(RuntimeType.LabelFormat(runtimeConfig, "fmt_string"))
rust("let $outputVar = $func($input, ${label.isGreedyLabel});")
val encodingStrategy = if (label.isGreedyLabel) {
RuntimeType.LabelFormat(runtimeConfig, "EncodingStrategy::Greedy")
} else {
RuntimeType.LabelFormat(runtimeConfig, "EncodingStrategy::Default")
}
rust("let $outputVar = $func($input, #T);", encodingStrategy)
}
target.isTimestampShape -> {
val timestampFormat =
Expand Down
31 changes: 23 additions & 8 deletions rust-runtime/aws-smithy-http/src/label.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,47 @@ use percent_encoding::AsciiSet;

const GREEDY: &AsciiSet = &BASE_SET.remove(b'/');

pub fn fmt_string<T: AsRef<str>>(t: T, greedy: bool) -> String {
let uri_set = if greedy { GREEDY } else { BASE_SET };
#[non_exhaustive]
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum EncodingStrategy {
Default,
Greedy,
}

pub fn fmt_string<T: AsRef<str>>(t: T, strategy: EncodingStrategy) -> String {
let uri_set = if strategy == EncodingStrategy::Greedy {
GREEDY
} else {
BASE_SET
};
percent_encoding::utf8_percent_encode(t.as_ref(), uri_set).to_string()
}

pub fn fmt_timestamp(t: &DateTime, format: Format) -> Result<String, DateTimeFormatError> {
Ok(crate::query::fmt_string(t.fmt(format)?))
Ok(fmt_string(t.fmt(format)?, EncodingStrategy::Default))
}

#[cfg(test)]
mod test {
use crate::label::fmt_string;
use crate::label::{fmt_string, EncodingStrategy};
use http::Uri;
use proptest::proptest;

#[test]
fn greedy_params() {
assert_eq!(fmt_string("a/b", false), "a%2Fb");
assert_eq!(fmt_string("a/b", true), "a/b");
assert_eq!(fmt_string("a/b", EncodingStrategy::Default), "a%2Fb");
assert_eq!(fmt_string("a/b", EncodingStrategy::Greedy), "a/b");
}

proptest! {
#[test]
fn test_encode_request(s: String) {
let _: Uri = format!("http://host.example.com/{}", fmt_string(&s, false)).parse().expect("all strings should be encoded properly");
let _: Uri = format!("http://host.example.com/{}", fmt_string(&s, true)).parse().expect("all strings should be encoded properly");
let _: Uri = format!("http://host.example.com/{}", fmt_string(&s, EncodingStrategy::Default))
.parse()
.expect("all strings should be encoded properly");
let _: Uri = format!("http://host.example.com/{}", fmt_string(&s, EncodingStrategy::Greedy))
.parse()
.expect("all strings should be encoded properly");
}
}
}
2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-http/src/urlencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use percent_encoding::{AsciiSet, CONTROLS};

/// base set of characters that must be URL encoded
pub const BASE_SET: &AsciiSet = &CONTROLS
pub(crate) const BASE_SET: &AsciiSet = &CONTROLS
.add(b' ')
.add(b'/')
// RFC-3986 §3.3 allows sub-delims (defined in section2.2) to be in the path component.
Expand Down