Only support T as an allowed separator in RFC-3339 dates (#3934)

This PR modifies `aws-smithy-types::DateTime` to enforce the use of 'T' (or 't') as the date-time separator. A [recent merge](https://github.com/time-rs/time/pull/700/files#diff-78a73b9911c88961601784e674a20c89e7050de9f8763522bbaa88ed28311b51R524) in the 'time' crate introduced support for space characters as separators. While ISO 8601 does allow spaces, RFC 3339 strictly requires 'T' (or 't') as the separator. This change restores the behavior to match how it was before `time = 0.3.37`. This PR also removes the temporary pinning of the time crate in protocol tests. Co-authored-by: Fahad Zubair <[email protected]>
smithy-lang · Dec 5, 2024 · 384f2ae · 384f2ae
1 parent 801054d
commit 384f2ae
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 5 deletions.
diff --git a/codegen-server-test/build.gradle.kts b/codegen-server-test/build.gradle.kts
@@ -21,8 +21,6 @@ val properties = PropertyRetriever(rootProject, project)
 val pluginName = "rust-server-codegen"
 val workingDirUnderBuildDir = "smithyprojections/codegen-server-test/"
 
-val checkedInSmithyRuntimeLockfile = rootProject.projectDir.resolve("rust-runtime/Cargo.lock")
-
 dependencies {
     implementation(project(":codegen-server"))
     implementation("software.amazon.smithy:smithy-aws-protocol-tests:$smithyVersion")
@@ -106,10 +104,9 @@ val allCodegenTests = "../codegen-core/common-test-models".let { commonModels ->
 project.registerGenerateSmithyBuildTask(rootProject, pluginName, allCodegenTests)
 project.registerGenerateCargoWorkspaceTask(rootProject, pluginName, allCodegenTests, workingDirUnderBuildDir)
 project.registerGenerateCargoConfigTomlTask(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile)
-project.registerCopyCheckedInCargoLockfileTask(checkedInSmithyRuntimeLockfile, layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile)
 
 tasks["smithyBuild"].dependsOn("generateSmithyBuild")
-tasks["assemble"].finalizedBy("generateCargoWorkspace", "copyCheckedInCargoLockfile", "generateCargoConfigToml")
+tasks["assemble"].finalizedBy("generateCargoWorkspace", "generateCargoConfigToml")
 
 project.registerModifyMtimeTask()
 project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile)

diff --git a/rust-runtime/aws-smithy-types/Cargo.toml b/rust-runtime/aws-smithy-types/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "aws-smithy-types"
-version = "1.2.9"
+version = "1.2.10"
 authors = [
     "AWS Rust SDK Team <[email protected]>",
     "Russell Cohen <[email protected]>",

diff --git a/rust-runtime/aws-smithy-types/src/date_time/format.rs b/rust-runtime/aws-smithy-types/src/date_time/format.rs
@@ -407,6 +407,12 @@ pub(crate) mod rfc3339 {
             )
             .into());
         }
+        if s.len() > 10 && !matches!(s.as_bytes()[10], b'T' | b't') {
+            return Err(DateTimeParseErrorKind::Invalid(
+                "RFC-3339 only allows `T` as a separator for date-time values".into(),
+            )
+            .into());
+        }
         let date_time = OffsetDateTime::parse(s, &Rfc3339).map_err(|err| {
             DateTimeParseErrorKind::Invalid(format!("invalid RFC-3339 date-time: {}", err).into())
         })?;
@@ -773,6 +779,43 @@ mod tests {
         http_date_check_roundtrip(9999999999, 0);
     }
 
+    #[test]
+    fn parse_rfc3339_invalid_separator() {
+        let test_cases = [
+            ("1985-04-12 23:20:50Z", AllowOffsets::OffsetsForbidden),
+            ("1985-04-12x23:20:50Z", AllowOffsets::OffsetsForbidden),
+            ("1985-04-12 23:20:50-02:00", AllowOffsets::OffsetsAllowed),
+            ("1985-04-12a23:20:50-02:00", AllowOffsets::OffsetsAllowed),
+        ];
+        for (date, offset) in test_cases.into_iter() {
+            let dt = rfc3339::parse(date, offset);
+            assert!(matches!(
+                dt.unwrap_err(),
+                DateTimeParseError {
+                    kind: DateTimeParseErrorKind::Invalid(_)
+                }
+            ));
+        }
+    }
+    #[test]
+    fn parse_rfc3339_t_separator() {
+        let test_cases = [
+            ("1985-04-12t23:20:50Z", AllowOffsets::OffsetsForbidden),
+            ("1985-04-12T23:20:50Z", AllowOffsets::OffsetsForbidden),
+            ("1985-04-12t23:20:50-02:00", AllowOffsets::OffsetsAllowed),
+            ("1985-04-12T23:20:50-02:00", AllowOffsets::OffsetsAllowed),
+        ];
+        for (date, offset) in test_cases.into_iter() {
+            let dt = rfc3339::parse(date, offset);
+            assert!(
+                dt.is_ok(),
+                "failed to parse date: '{}' with error: {:?}",
+                date,
+                dt.err().unwrap()
+            );
+        }
+    }
+
     proptest! {
         #![proptest_config(ProptestConfig::with_cases(10000))]