Skip to content

Commit

Permalink
fix datetime parsing on server and clients
Browse files Browse the repository at this point in the history
  • Loading branch information
rcoh committed Feb 6, 2023
1 parent 265e100 commit bf20cec
Show file tree
Hide file tree
Showing 12 changed files with 96 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ class RequestBindingGenerator(
target.isTimestampShape -> {
val timestampFormat =
index.determineTimestampFormat(member, HttpBinding.Location.QUERY, protocol.defaultTimestampFormat)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.serializeTimestampFormat(runtimeConfig, timestampFormat)
val func = writer.format(RuntimeType.queryFormat(runtimeConfig, "fmt_timestamp"))
"&$func($targetName, ${writer.format(timestampFormatType)})?"
}
Expand Down Expand Up @@ -318,7 +318,7 @@ class RequestBindingGenerator(
target.isTimestampShape -> {
val timestampFormat =
index.determineTimestampFormat(member, HttpBinding.Location.LABEL, protocol.defaultTimestampFormat)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.serializeTimestampFormat(runtimeConfig, timestampFormat)
val func = format(RuntimeType.labelFormat(runtimeConfig, "fmt_timestamp"))
rust("let $outputVar = $func($input, ${format(timestampFormatType)})?;")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,29 @@ data class RuntimeType(val path: String, val dependency: RustDependency? = null)
fun sdkSuccess(runtimeConfig: RuntimeConfig): RuntimeType =
smithyHttp(runtimeConfig).resolve("result::SdkSuccess")

fun timestampFormat(runtimeConfig: RuntimeConfig, format: TimestampFormatTrait.Format): RuntimeType {
fun parseTimestampFormat(
codegenTarget: CodegenTarget,
runtimeConfig: RuntimeConfig,
format: TimestampFormatTrait.Format,
): RuntimeType {
val timestampFormat = when (format) {
TimestampFormatTrait.Format.EPOCH_SECONDS -> "EpochSeconds"
// clients allow offsets, servers do nt
TimestampFormatTrait.Format.DATE_TIME -> codegenTarget.ifClient { "DateTimeWithOffset" } ?: "DateTime"
TimestampFormatTrait.Format.HTTP_DATE -> "HttpDate"
TimestampFormatTrait.Format.UNKNOWN -> TODO()
}

return smithyTypes(runtimeConfig).resolve("date_time::Format::$timestampFormat")
}

fun serializeTimestampFormat(
runtimeConfig: RuntimeConfig,
format: TimestampFormatTrait.Format,
): RuntimeType {
val timestampFormat = when (format) {
TimestampFormatTrait.Format.EPOCH_SECONDS -> "EpochSeconds"
// clients allow offsets, servers do not
TimestampFormatTrait.Format.DATE_TIME -> "DateTime"
TimestampFormatTrait.Format.HTTP_DATE -> "HttpDate"
TimestampFormatTrait.Format.UNKNOWN -> TODO()
Expand All @@ -311,8 +331,7 @@ data class RuntimeType(val path: String, val dependency: RustDependency? = null)
fun captureRequest(runtimeConfig: RuntimeConfig) =
CargoDependency.smithyClientTestUtil(runtimeConfig).toType().resolve("test_connection::capture_request")

fun forInlineDependency(inlineDependency: InlineDependency) =
RuntimeType("crate::${inlineDependency.name}", inlineDependency)
fun forInlineDependency(inlineDependency: InlineDependency) = RuntimeType("crate::${inlineDependency.name}", inlineDependency)

fun forInlineFun(name: String, module: RustModule, func: Writable) = RuntimeType(
"${module.fullyQualifiedPath()}::$name",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ class HttpBindingGenerator(
HttpBinding.Location.HEADER,
defaultTimestampFormat,
)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.parseTimestampFormat(codegenTarget, runtimeConfig, timestampFormat)
rust(
"let $parsedValue: Vec<${coreType.render()}> = #T::many_dates(headers, #T)?;",
headerUtil,
Expand Down Expand Up @@ -753,7 +753,7 @@ class HttpBindingGenerator(
}

target.isTimestampShape -> {
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.serializeTimestampFormat(runtimeConfig, timestampFormat)
quoteValue("$targetName.fmt(${writer.format(timestampFormatType)})?")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ typealias JsonParserCustomization = NamedCustomization<JsonParserSection>
data class ReturnSymbolToParse(val symbol: Symbol, val isUnconstrained: Boolean)

class JsonParserGenerator(
codegenContext: CodegenContext,
private val codegenContext: CodegenContext,
private val httpBindingResolver: HttpBindingResolver,
/** Function that maps a MemberShape into a JSON field name */
private val jsonName: (MemberShape) -> String,
Expand Down Expand Up @@ -347,7 +347,7 @@ class JsonParserGenerator(
member, HttpLocation.DOCUMENT,
TimestampFormatTrait.Format.EPOCH_SECONDS, model,
)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.parseTimestampFormat(codegenTarget, runtimeConfig, timestampFormat)
rustTemplate(
"#{expect_timestamp_or_null}(tokens.next(), #{T})?#{ConvertFrom:W}",
"T" to timestampFormatType, "ConvertFrom" to typeConversionGenerator.convertViaFrom(shape), *codegenScope,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ class XmlBindingTraitParserGenerator(

private val scopedDecoder = smithyXml.resolve("decode::ScopedDecoder")
private val runtimeConfig = codegenContext.runtimeConfig
private val codegenTarget = codegenContext.target

// The symbols we want all the time
private val codegenScope = arrayOf(
Expand Down Expand Up @@ -638,7 +639,7 @@ class XmlBindingTraitParserGenerator(
HttpBinding.Location.DOCUMENT,
TimestampFormatTrait.Format.DATE_TIME,
)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.parseTimestampFormat(codegenTarget, runtimeConfig, timestampFormat)
withBlock("#T::from_str(", ")", RuntimeType.dateTime(runtimeConfig)) {
provider()
rust(", #T", timestampFormatType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ class JsonSerializerGenerator(
is TimestampShape -> {
val timestampFormat =
httpBindingResolver.timestampFormat(context.shape, HttpLocation.DOCUMENT, EPOCH_SECONDS, model)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.serializeTimestampFormat(runtimeConfig, timestampFormat)
rustTemplate(
"$writer.date_time(${value.asRef()}#{ConvertInto:W}, #{FormatType})?;",
"FormatType" to timestampFormatType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ abstract class QuerySerializerGenerator(codegenContext: CodegenContext) : Struct
)
is TimestampShape -> {
val timestampFormat = determineTimestampFormat(context.shape)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.serializeTimestampFormat(runtimeConfig, timestampFormat)
rust("$writer.date_time(${value.name}, #T)?;", timestampFormatType)
}
is CollectionShape -> serializeCollection(context, Context(writer, context.valueExpression, target))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ class XmlBindingTraitSerializerGenerator(
HttpLocation.DOCUMENT,
TimestampFormatTrait.Format.DATE_TIME, model,
)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.parseTimestampFormat(codegenTarget, runtimeConfig, timestampFormat)
rust("$input.fmt(#T)?.as_ref()", timestampFormatType)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.stripOuter
import software.amazon.smithy.rust.codegen.core.rustlang.withBlock
import software.amazon.smithy.rust.codegen.core.rustlang.withBlockTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationCustomization
import software.amazon.smithy.rust.codegen.core.smithy.generators.TypeConversionGenerator
Expand Down Expand Up @@ -935,7 +936,7 @@ private class ServerHttpBoundProtocolTraitImplGenerator(
it.location,
protocol.defaultTimestampFormat,
)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.parseTimestampFormat(CodegenTarget.SERVER, runtimeConfig, timestampFormat)
rustTemplate(
"""
let v = #{DateTime}::from_str(&v, #{format})?#{ConvertInto:W};
Expand Down Expand Up @@ -1088,7 +1089,7 @@ private class ServerHttpBoundProtocolTraitImplGenerator(
binding.location,
protocol.defaultTimestampFormat,
)
val timestampFormatType = RuntimeType.timestampFormat(runtimeConfig, timestampFormat)
val timestampFormatType = RuntimeType.parseTimestampFormat(CodegenTarget.SERVER, runtimeConfig, timestampFormat)

if (percentDecoding) {
rustTemplate(
Expand Down
10 changes: 6 additions & 4 deletions rust-runtime/aws-smithy-json/src/deserialize/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,12 @@ pub fn expect_timestamp_or_null(
}
})
.transpose()?,
Format::DateTime | Format::HttpDate => expect_string_or_null(token)?
.map(|v| DateTime::from_str(v.as_escaped_str(), timestamp_format))
.transpose()
.map_err(|err| Error::custom_source("failed to parse timestamp", err))?,
Format::DateTime | Format::HttpDate | Format::DateTimeWithOffset => {
expect_string_or_null(token)?
.map(|v| DateTime::from_str(v.as_escaped_str(), timestamp_format))
.transpose()
.map_err(|err| Error::custom_source("failed to parse timestamp", err))?
}
})
}

Expand Down
49 changes: 42 additions & 7 deletions rust-runtime/aws-smithy-types/src/date_time/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,12 +399,28 @@ pub(crate) mod rfc3339 {
use time::format_description::well_known::Rfc3339;
use time::OffsetDateTime;

#[derive(Debug, PartialEq)]
pub(crate) enum AllowOffsets {
OffsetsAllowed,
OffsetsForbidden,
}

// OK: 1985-04-12T23:20:50.52Z
// OK: 1985-04-12T23:20:50Z
//
// Timezones not supported:
// Not OK: 1985-04-12T23:20:50-02:00
pub(crate) fn parse(s: &str) -> Result<DateTime, DateTimeParseError> {
pub(crate) fn parse(
s: &str,
allow_offsets: AllowOffsets,
) -> Result<DateTime, DateTimeParseError> {
if allow_offsets == AllowOffsets::OffsetsForbidden && !matches!(s.chars().last(), Some('Z'))
{
return Err(DateTimeParseErrorKind::Invalid(
"Smithy does not support timezone offsets in RFC-3339 date times".into(),
)
.into());
}
let date_time = OffsetDateTime::parse(s, &Rfc3339).map_err(|err| {
DateTimeParseErrorKind::Invalid(format!("invalid RFC-3339 date-time: {}", err).into())
})?;
Expand All @@ -413,10 +429,13 @@ pub(crate) mod rfc3339 {
}

/// Read 1 RFC-3339 date from &str and return the remaining str
pub(crate) fn read(s: &str) -> Result<(DateTime, &str), DateTimeParseError> {
pub(crate) fn read(
s: &str,
allow_offests: AllowOffsets,
) -> Result<(DateTime, &str), DateTimeParseError> {
let delim = s.find('Z').map(|idx| idx + 1).unwrap_or_else(|| s.len());
let (head, rest) = s.split_at(delim);
Ok((parse(head)?, rest))
Ok((parse(head, allow_offests)?, rest))
}

/// Format a [DateTime] in the RFC-3339 date format
Expand Down Expand Up @@ -485,6 +504,7 @@ pub(crate) mod rfc3339 {
#[cfg(test)]
mod tests {
use super::*;
use crate::date_time::format::rfc3339::AllowOffsets;
use crate::DateTime;
use lazy_static::lazy_static;
use proptest::prelude::*;
Expand Down Expand Up @@ -628,7 +648,9 @@ mod tests {

#[test]
fn parse_date_time() {
parse_test(&TEST_CASES.parse_date_time, rfc3339::parse);
parse_test(&TEST_CASES.parse_date_time, |date| {
rfc3339::parse(date, AllowOffsets::OffsetsForbidden)
});
}

#[test]
Expand All @@ -649,8 +671,10 @@ mod tests {
#[test]
fn read_rfc3339_date_comma_split() {
let date = "1985-04-12T23:20:50Z,1985-04-12T23:20:51Z";
let (e1, date) = rfc3339::read(date).expect("should succeed");
let (e2, date2) = rfc3339::read(&date[1..]).expect("should succeed");
let (e1, date) =
rfc3339::read(date, AllowOffsets::OffsetsForbidden).expect("should succeed");
let (e2, date2) =
rfc3339::read(&date[1..], AllowOffsets::OffsetsForbidden).expect("should succeed");
assert_eq!(date2, "");
assert_eq!(date, ",1985-04-12T23:20:51Z");
let expected = DateTime::from_secs_and_nanos(482196050, 0);
Expand All @@ -661,10 +685,21 @@ mod tests {

#[test]
fn parse_rfc3339_with_timezone() {
let dt = rfc3339::parse("1985-04-12T21:20:51-02:00");
let dt = rfc3339::parse("1985-04-12T21:20:51-02:00", AllowOffsets::OffsetsAllowed);
assert_eq!(dt.unwrap(), DateTime::from_secs_and_nanos(482196051, 0));
}

#[test]
fn parse_rfc3339_timezone_forbidden() {
let dt = rfc3339::parse("1985-04-12T23:20:50-02:00", AllowOffsets::OffsetsForbidden);
assert!(matches!(
dt.unwrap_err(),
DateTimeParseError {
kind: DateTimeParseErrorKind::Invalid(_)
}
));
}

#[test]
fn http_date_out_of_range() {
assert_eq!(
Expand Down
16 changes: 12 additions & 4 deletions rust-runtime/aws-smithy-types/src/date_time/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

//! DateTime type for representing Smithy timestamps.
use crate::date_time::format::rfc3339::AllowOffsets;
use crate::date_time::format::DateTimeParseErrorKind;
use num_integer::div_mod_floor;
use num_integer::Integer;
Expand Down Expand Up @@ -155,7 +156,8 @@ impl DateTime {
/// Parses a `DateTime` from a string using the given `format`.
pub fn from_str(s: &str, format: Format) -> Result<Self, DateTimeParseError> {
match format {
Format::DateTime => format::rfc3339::parse(s),
Format::DateTime => format::rfc3339::parse(s, AllowOffsets::OffsetsForbidden),
Format::DateTimeWithOffset => format::rfc3339::parse(s, AllowOffsets::OffsetsAllowed),
Format::HttpDate => format::http_date::parse(s),
Format::EpochSeconds => format::epoch_seconds::parse(s),
}
Expand Down Expand Up @@ -207,7 +209,8 @@ impl DateTime {
/// Enable parsing multiple dates from the same string
pub fn read(s: &str, format: Format, delim: char) -> Result<(Self, &str), DateTimeParseError> {
let (inst, next) = match format {
Format::DateTime => format::rfc3339::read(s)?,
Format::DateTime => format::rfc3339::read(s, AllowOffsets::OffsetsForbidden)?,
Format::DateTimeWithOffset => format::rfc3339::read(s, AllowOffsets::OffsetsAllowed)?,
Format::HttpDate => format::http_date::read(s)?,
Format::EpochSeconds => {
let split_point = s.find(delim).unwrap_or(s.len());
Expand All @@ -229,7 +232,7 @@ impl DateTime {
/// Returns an error if the given `DateTime` cannot be represented by the desired format.
pub fn fmt(&self, format: Format) -> Result<String, DateTimeFormatError> {
match format {
Format::DateTime => format::rfc3339::format(self),
Format::DateTime | Format::DateTimeWithOffset => format::rfc3339::format(self),
Format::EpochSeconds => Ok(format::epoch_seconds::format(self)),
Format::HttpDate => format::http_date::format(self),
}
Expand Down Expand Up @@ -314,10 +317,15 @@ impl fmt::Display for ConversionError {
/// Formats for representing a `DateTime` in the Smithy protocols.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum Format {
/// RFC-3339 Date Time.
/// RFC-3339 Date Time. If the date time has an offset, an error will be returned
DateTime,

/// RFC-3339 Date Time. Offsets are supported
DateTimeWithOffset,

/// Date format used by the HTTP `Date` header, specified in RFC-7231.
HttpDate,

/// Number of seconds since the Unix epoch formatted as a floating point.
EpochSeconds,
}
Expand Down

0 comments on commit bf20cec

Please sign in to comment.