Fix bug where paginators looped forever on empty string next token

aws/sdk/integration-tests/Cargo.toml

@@ -3,6 +3,7 @@
 [workspace]
 members = [
     "dynamodb",
+    "ec2",
     "iam",
     "kms",
     "polly",

aws/sdk/integration-tests/dynamodb/tests/paginators.rs

@@ -53,7 +53,7 @@ fn mk_response(body: &'static str) -> http::Response<SdkBody> {
     http::Response::builder().body(SdkBody::from(body)).unwrap()
 }
 
-#[tokio::test]
+#[tokio::test(flavor = "current_thread")]
 async fn paginators_loop_until_completion() {
     let conn = TestConnection::new(vec![
         (
@@ -169,3 +169,54 @@ async fn paginators_handle_errors() {
     rows.try_next().await.expect_err("failure");
     assert_eq!(rows.try_next().await.expect("ok"), None);
 }
+
+#[tokio::test]
+async fn paginators_error_on_repeated_token() {
+    let response = r#"{
+        "Count": 1,
+        "Items": [{
+            "PostedBy": {
+                "S": "joe@example.com"
+            }
+        }],
+        "LastEvaluatedKey": {
+            "PostedBy": { "S": "joe@example.com" }
+        }
+    }"#;
+    // send the same response twice with the same pagination token
+    let conn = TestConnection::new(vec![
+        (
+            mk_request(r#"{"TableName":"test-table","Limit":32}"#),
+            mk_response(response),
+        ),
+        (
+            mk_request(
+                r#"{"TableName":"test-table","Limit":32,"ExclusiveStartKey":{"PostedBy":{"S":"joe@example.com"}}}"#,
+            ),
+            mk_response(response),
+        ),
+    ]);
+    let client = Client::from_conf_conn(stub_config(), conn.clone());
+    let mut rows = client
+        .scan()
+        .table_name("test-table")
+        .into_paginator()
+        .page_size(32)
+        .items()
+        .send();
+    assert_eq!(
+        rows.try_next()
+            .await
+            .expect("no error")
+            .expect("not EOS")
+            .get("PostedBy"),
+        Some(&AttributeValue::S("joe@example.com".to_string()))
+    );
+    let err = rows.try_next().await.expect_err("failure");
+    assert!(
+        format!("{}", err).contains("next token did not change"),
+        "{}",
+        err
+    );
+    assert_eq!(rows.try_next().await.expect("ok"), None);
+}

aws/sdk/integration-tests/ec2/Cargo.toml

@@ -0,0 +1,11 @@
+[package]
+name = "ec2-tests"
+version = "0.1.0"
+edition = "2018"
+
+[dev-dependencies]
+aws-sdk-ec2 = { path = "../../build/aws-sdk/sdk/ec2" }
+aws-smithy-client = { path = "../../build/aws-sdk/sdk/aws-smithy-client", features = ["test-util"]}
+tokio = { version = "1", features = ["full"]}
+http = "0.2.6"
+tokio-stream = "0.1.8"

aws/sdk/integration-tests/ec2/src/lib.rs

@@ -0,0 +1,4 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0.
+ */

aws/sdk/integration-tests/ec2/tests/paginators.rs

@@ -0,0 +1,52 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0.
+ */
+
+use aws_sdk_ec2::{model::InstanceType, Client, Config, Credentials, Region};
+use aws_smithy_client::test_connection::TestConnection;
+use tokio_stream::StreamExt;
+
+fn stub_config() -> Config {
+    Config::builder()
+        .region(Region::new("us-east-1"))
+        .credentials_provider(Credentials::new("akid", "secret", None, None, "test"))
+        .build()
+}
+
+/// See https://github.com/awslabs/aws-sdk-rust/issues/391
+///
+/// EC2 replies with `<nextToken></nextToken>` which our XML parser parses as empty string and not "none"
+#[tokio::test]
+async fn paginators_handle_empty_tokens() {
+    let request= "Action=DescribeSpotPriceHistory&Version=2016-11-15&AvailabilityZone=eu-north-1a&InstanceType.1=g5.48xlarge&ProductDescription.1=Linux%2FUNIX";
+    let response = r#"<?xml version="1.0" encoding="UTF-8"?>
+        <DescribeSpotPriceHistoryResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/">
+            <requestId>edf3e86c-4baf-47c1-9228-9a5ea09542e8</requestId>
+            <spotPriceHistorySet/>
+            <nextToken></nextToken>
+        </DescribeSpotPriceHistoryResponse>"#;
+    let conn = TestConnection::<&str>::new(vec![(
+        http::Request::builder()
+            .uri("https://ec2.us-east-1.amazonaws.com/")
+            .body(request.into())
+            .unwrap(),
+        http::Response::builder()
+            .status(200)
+            .body(response)
+            .unwrap(),
+    )]);
+    let client = Client::from_conf_conn(stub_config(), conn.clone());
+    let instance_type = InstanceType::from("g5.48xlarge");
+    let mut paginator = client
+        .describe_spot_price_history()
+        .instance_types(instance_type)
+        .product_descriptions("Linux/UNIX")
+        .availability_zone("eu-north-1a")
+        .into_paginator()
+        .items()
+        .send();
+    let first_item = paginator.try_next().await.expect("success");
+    assert_eq!(first_item, None);
+    conn.assert_requests_match(&[]);
+}

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/PaginatorGenerator.kt

@@ -9,6 +9,7 @@ import software.amazon.smithy.model.Model
 import software.amazon.smithy.model.knowledge.PaginatedIndex
 import software.amazon.smithy.model.shapes.OperationShape
 import software.amazon.smithy.model.shapes.ServiceShape
+import software.amazon.smithy.model.shapes.StringShape
 import software.amazon.smithy.model.traits.IdempotencyTokenTrait
 import software.amazon.smithy.model.traits.PaginatedTrait
 import software.amazon.smithy.rust.codegen.rustlang.CargoDependency
@@ -173,8 +174,13 @@ class PaginatorGenerator private constructor(
                             // If the input member is None or it was an error
                             let done = match resp {
                                 Ok(ref resp) => {
-                                    input.$inputTokenMember = #{output_token}(resp).cloned();
-                                    input.$inputTokenMember.is_none()
+                                    let new_token = #{output_token}(resp);
+                                    if new_token == input.$inputTokenMember.as_ref() {
+                                        let _ = tx.send(Err(#{SdkError}::ConstructionFailure("next token did not change, aborting paginator. This indicates an SDK or AWS service bug.".into()))).await;
+                                        return;
+                                    }
+                                    input.$inputTokenMember = new_token.cloned();
+                                    ${nextTokenEmpty("input.$inputTokenMember")}
                                 },
                                 Err(_) => true,
                             };
@@ -269,6 +275,15 @@ class PaginatorGenerator private constructor(
         }
     }
 
+    private fun nextTokenEmpty(token: String): String {
+        val tokenType = model.expectShape(paginationInfo.outputTokenMemberPath.last().target)
+        if (tokenType is StringShape) {
+            return "$token.as_deref().unwrap_or_default().is_empty()"
+        } else {
+            return "$token.is_none()"
+        }
+    }
+
     private fun pageSizeSetter() = writable {
         paginationInfo.pageSizeMember.orNull()?.also {
             val memberName = symbolProvider.toMemberName(it)

codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/PaginatorGeneratorTest.kt

@@ -67,7 +67,7 @@ internal class PaginatorGeneratorTest {
     """.asSmithyModel()
 
     @Test
-    fun `generate correct paginators`() {
+    fun `generate paginators that compile`() {
         val (ctx, testDir) = generatePluginContext(model)
         RustCodegenPlugin().execute(ctx)
         "cargo test".runCommand(testDir)