refactor(extras): drop the redirect target text

packages/aws-durable-execution-sdk-python-extras/README.md

@@ -136,7 +136,7 @@ A step retry costs one durable operation and one more Lambda invocation. Most fa
 1. **Inside one step attempt.** The boto3 client retries throttling and server errors of `RunMicrovm`, `CreateMicrovmAuthToken`, and `TerminateMicrovm`. The request step retries the POST itself: connection errors, timeouts, 429, and 5xx, with a delay from 250 milliseconds to 4 seconds. It retries one 401 or 403 with a new auth token. The endpoint answers this way until the MicroVM's `run` hook returns. This tier ends after `retry_window`, or 10 seconds before the invocation times out, whichever comes first.
 2. **The step retry strategy.** When the first tier gives up, the step retries after a longer delay, outside the invocation. `default_microvm_retry_strategy` is the default, unless the config sets `retry_strategy`. It retries throttling, server errors, connection failures, timeouts, and the end of the first tier, for at most 5 attempts, with a delay of 2, 4, 8, and 16 seconds. Every other error, such as `ValidationException`, `AccessDeniedException`, or `InsufficientCapacityException`, fails the step at once.
 
-A route that answers another 4xx status fails the operation without a retry, with `MicrovmDeliveryError`. So does a redirect, 3xx. The operation does not follow redirects, and the message names the `Location`. Set the path that the route serves, for example with its trailing slash. The JavaScript operation also fails every 3xx at once, with `MicrovmDeliveryError`.
+A route that answers another 4xx status fails the operation without a retry, with `MicrovmDeliveryError`. So does a redirect, 3xx. The operation does not follow redirects. The execution history stores the error message. A query in the `Location` can hold a secret. So the message names the path, and the status of a 3xx answer. It omits the `Location`, and the URL that a followed redirect reached. Set the path that the route serves, for example with its trailing slash. The JavaScript operation also fails every 3xx at once, with `MicrovmDeliveryError`.
 
 The POST ignores the proxy settings in the environment, as the JavaScript SDK's `fetch` does. boto3 reads `HTTPS_PROXY`. So in a network that reaches the MicroVM endpoint only through a proxy, the launch succeeds and every POST fails. Pass an `opener` in the config for that network, for example `urllib.request.build_opener(urllib.request.ProxyHandler({"https": "http://proxy.internal:3128"}))`. When an opener follows a redirect, the delivery still fails at once with `MicrovmDeliveryError`. The followed request carries no auth token, because the operation adds its headers as unredirected headers.
 

packages/aws-durable-execution-sdk-python-extras/src/aws_durable_execution_sdk_python_extras/microvm/request.py

@@ -283,10 +283,14 @@ def send_job(options: SendJobOptions) -> int:
     The function does not follow a redirect, and neither does the JavaScript
     operation. A followed 301, 302, or 303 would turn the POST into a GET
     without the job. A followed 307 or 308 to another host would receive the
-    auth token. So a redirect fails the delivery at once, with the
-    ``Location`` in the message. The headers are added as unredirected
-    headers, so an opener that follows a redirect does not send the auth
-    token on either.
+    auth token. So a redirect fails the delivery at once. The headers are
+    added as unredirected headers, so an opener that follows a redirect does
+    not send the auth token on either.
+
+    The execution history stores the message. A query in the ``Location``
+    can hold a secret, for example a presigned signature. So the message
+    names the path, and the status of a 3xx answer. It omits the
+    ``Location``, and the URL that a followed redirect reached.
 
     The delay starts at 250 milliseconds and doubles up to 4 seconds. The
     tier ends at the retry window, or 10 seconds before the invocation times
@@ -396,9 +400,9 @@ def new_token() -> Mapping[str, str]:
             # the job: a 301, 302, or 303 turns the POST into a GET.
             msg = (
                 f"The MicroVM answered the job at {options.path} with a "
-                f"redirect to {answer.location}, and the opener followed it. "
-                "The operation does not follow redirects, so set the path that "
-                "the route serves."
+                "redirect, and the opener followed it. The operation does not "
+                "follow redirects, so set the path that the route serves, and "
+                "pass an opener that follows no redirect."
             )
             raise MicrovmRequestRejectedError(msg, status or 0)
 
@@ -429,11 +433,13 @@ def new_token() -> Mapping[str, str]:
                 continue
             if 300 <= status < 400:  # noqa: PLR2004
                 msg = (
-                    f"The MicroVM answered the job at {options.path} with HTTP "
-                    f"{status}, a redirect to {answer.location or 'no location'}. "
-                    "The operation does not follow redirects, so set the path "
-                    "that the route serves."
+                    f"The MicroVM answered the job at {options.path} with HTTP {status}"
                 )
+                if answer.has_location:
+                    msg += (
+                        ", a redirect. The operation does not follow redirects, "
+                        "so set the path that the route serves."
+                    )
                 raise MicrovmRequestRejectedError(msg, status)
             if 400 <= status < 500 and status != 429:  # noqa: PLR2004
                 msg = (
@@ -535,9 +541,8 @@ class _Answer:
     """The HTTP status. ``None`` when no answer arrived."""
     failure: str = ""
     """Why no answer arrived."""
-    location: str | None = None
-    """The ``Location`` header of a redirect, or the URL that a followed
-    redirect reached."""
+    has_location: bool = False
+    """Whether the answer has a ``Location`` header."""
     followed_redirect: bool = False
     """Whether the opener followed a redirect."""
 
@@ -587,14 +592,12 @@ def _post(
         with opener.open(request, timeout=timeout) as response:
             # An opener that follows redirects reports the URL it reached.
             if response.geturl() != url:
-                return _Answer(
-                    response.status, location=response.geturl(), followed_redirect=True
-                )
+                return _Answer(response.status, followed_redirect=True)
             return _Answer(response.status)
     except urllib.error.HTTPError as error:
-        location = error.headers.get("Location") if error.headers else None
+        has_location = bool(error.headers and error.headers.get("Location"))
         error.close()
-        return _Answer(error.code, location=location)
+        return _Answer(error.code, has_location=has_location)
     except (OSError, http.client.HTTPException, ValueError) as error:
         # URLError, a refused connection, and a timeout are OSError. An
         # invalid response from the endpoint raises an HTTPException.

packages/aws-durable-execution-sdk-python-extras/tests/conftest.py

@@ -141,11 +141,13 @@ class FakeEndpoint:
     It records each request, and answers from ``statuses`` in order. After
     the list ends, it answers 202. A status of ``None`` closes the connection
     without an answer. A redirect status gets ``Location: <location>``,
-    ``/moved`` by default.
+    ``/moved`` by default. A ``location`` of ``None`` sends no ``Location``.
     """
 
     def __init__(
-        self, statuses: list[int | None] | None = None, location: str = "/moved"
+        self,
+        statuses: list[int | None] | None = None,
+        location: str | None = "/moved",
     ) -> None:
         self.requests: list[dict[str, Any]] = []
         self._statuses = list(statuses or [])
@@ -185,7 +187,7 @@ def do_POST(self) -> None:  # noqa: N802 - the http.server name
                     self.connection.shutdown(socket.SHUT_RDWR)
                     return
                 self.send_response(status)
-                if 300 <= status < 400:  # noqa: PLR2004
+                if 300 <= status < 400 and location is not None:  # noqa: PLR2004
                     self.send_header("location", location)
                 self.send_header("content-length", "0")
                 self.end_headers()
@@ -211,7 +213,7 @@ def fake_endpoint() -> Iterator[Callable[..., FakeEndpoint]]:
     endpoints: list[FakeEndpoint] = []
 
     def _create(
-        statuses: list[int | None] | None = None, location: str = "/moved"
+        statuses: list[int | None] | None = None, location: str | None = "/moved"
     ) -> FakeEndpoint:
         endpoint = FakeEndpoint(statuses, location)
         endpoints.append(endpoint)

packages/aws-durable-execution-sdk-python-extras/tests/microvm/request_test.py

@@ -383,13 +383,57 @@ def test_rejects_a_redirect_without_following_it(
     assert raised.value.status == status
     assert str(raised.value) == (
         f"The MicroVM answered the job at /durable-execution/v1/job with HTTP "
-        f"{status}, a redirect to /moved. The operation does not follow "
-        "redirects, so set the path that the route serves."
+        f"{status}, a redirect. The operation does not follow redirects, so set "
+        "the path that the route serves."
     )
     # A followed 301, 302, or 303 would arrive as a GET without the job.
     assert [r["path"] for r in endpoint.requests] == ["/durable-execution/v1/job"]
 
 
+@pytest.mark.parametrize("status", [302, 304])
+def test_rejects_a_3xx_without_a_location(
+    fake_client: Any, fake_endpoint: Any, status: int
+) -> None:
+    endpoint = fake_endpoint([status], location=None)
+
+    with pytest.raises(MicrovmRequestRejectedError) as raised:
+        send_job(options(fake_client(), endpoint.url, []))
+
+    assert raised.value.status == status
+    assert str(raised.value) == (
+        f"The MicroVM answered the job at /durable-execution/v1/job with HTTP {status}"
+    )
+
+
+def test_rejects_a_3xx_with_an_empty_location(
+    fake_client: Any, fake_endpoint: Any
+) -> None:
+    # An empty Location names no target. So it counts as no Location.
+    endpoint = fake_endpoint([302], location="")
+
+    with pytest.raises(MicrovmRequestRejectedError) as raised:
+        send_job(options(fake_client(), endpoint.url, []))
+
+    assert raised.value.status == 302
+    assert str(raised.value) == (
+        "The MicroVM answered the job at /durable-execution/v1/job with HTTP 302"
+    )
+
+
+def test_omits_the_location_of_a_redirect_from_the_message(
+    fake_client: Any, fake_endpoint: Any
+) -> None:
+    # The execution history stores the message. A query in the Location can
+    # hold a secret. So the message must not contain any part of it.
+    endpoint = fake_endpoint([302], location="https://example.com/x?token=secret")
+
+    with pytest.raises(MicrovmRequestRejectedError) as raised:
+        send_job(options(fake_client(), endpoint.url, []))
+
+    assert "example.com" not in str(raised.value)
+    assert "secret" not in str(raised.value)
+
+
 @pytest.mark.parametrize(
     "path", ["/clone%20build", "/caf%C3%A9", "/a/b-c_d.e~f!$&'()*+,;=:@", "/a/"]
 )
@@ -439,16 +483,35 @@ def test_rejects_a_redirect_that_the_callers_opener_followed(
 
     assert str(raised.value) == (
         "The MicroVM answered the job at /durable-execution/v1/job with a "
-        f"redirect to {endpoint.url}/moved, and the opener followed it. The "
-        "operation does not follow redirects, so set the path that the route "
-        "serves."
+        "redirect, and the opener followed it. The operation does not follow "
+        "redirects, so set the path that the route serves, and pass an opener "
+        "that follows no redirect."
     )
     assert [r["path"] for r in endpoint.requests] == [
         "/durable-execution/v1/job",
         "/moved",
     ]
 
 
+def test_omits_the_followed_url_from_the_message(
+    fake_client: Any, fake_endpoint: Any
+) -> None:
+    # The opener follows the 302 to the URL with the query. The message must
+    # not contain that URL.
+    endpoint = fake_endpoint([302], location="/x?token=secret")
+
+    with pytest.raises(MicrovmRequestRejectedError) as raised:
+        send_job(
+            options(
+                fake_client(), endpoint.url, [], opener=urllib.request.build_opener()
+            )
+        )
+
+    assert endpoint.requests[-1]["path"] == "/x?token=secret"
+    assert "?token=" not in str(raised.value)
+    assert "secret" not in str(raised.value)
+
+
 def test_builds_the_default_opener_once() -> None:
     assert request.default_opener() is request.default_opener()