Skip to content

Commit cc07744

Browse files
committed
update doc and add test
1 parent 97d292f commit cc07744

2 files changed

Lines changed: 153 additions & 42 deletions

File tree

awscrt/http.py

Lines changed: 29 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,28 @@ def request(self,
325325
This allows calling :meth:`HttpClientStream.write_data()` to stream
326326
the request body in chunks. Works for both HTTP/1.1 and HTTP/2.
327327
328+
By design, CRT does not support setting both a body stream and
329+
enabling manual writes for HTTP/1.1. Body streams are intended
330+
for requests whose payload is available at the time of sending.
331+
Manual writes let the caller control when data is sent. The two
332+
cannot coexist on HTTP/1.1.
333+
334+
If ``manual_write`` is True and ``request`` has a ``body_stream``,
335+
this method raises :class:`ValueError`.
336+
337+
HTTP/2 does not have this restriction.
338+
328339
Returns:
329340
HttpClientStream:
341+
342+
Raises:
343+
ValueError: If ``manual_write`` is True and the request has a
344+
``body_stream`` set (HTTP/1.1 only).
330345
"""
346+
if manual_write and request.body_stream is not None:
347+
raise ValueError(
348+
"Cannot use manual data writes with a body_stream on an HTTP/1.1 request. "
349+
"Either remove the body_stream or set manual_write=False.")
331350
return HttpClientStream(self, request, on_response, on_body, manual_write)
332351

333352
def close(self) -> "concurrent.futures.Future":
@@ -531,7 +550,7 @@ def _init_common(self,
531550
request: 'HttpRequest',
532551
on_response: Optional[Callable[..., None]] = None,
533552
on_body: Optional[Callable[..., None]] = None,
534-
http2_manual_write: bool = False) -> None:
553+
manual_write: bool = False) -> None:
535554
assert isinstance(connection, HttpClientConnectionBase)
536555
assert isinstance(request, HttpRequest)
537556
assert callable(on_response) or on_response is None
@@ -545,7 +564,7 @@ def _init_common(self,
545564
# keep HttpRequest alive until stream completes
546565
self._request = request
547566
self._version = connection.version
548-
self._binding = _awscrt.http_client_stream_new(self, connection, request, http2_manual_write)
567+
self._binding = _awscrt.http_client_stream_new(self, connection, request, manual_write)
549568

550569
@property
551570
def version(self) -> HttpVersion:
@@ -676,20 +695,9 @@ def write_data(self,
676695
The stream must have been created with ``manual_write=True`` and
677696
:meth:`activate()` must have been called before using this method.
678697
679-
.. important::
680-
HTTP/1.1 does **not** support combining a body stream with manual
681-
writes. The underlying H1 encoder uses a modal state machine that
682-
commits to a single body-delivery path at initialisation time.
683-
If the :class:`HttpRequest` was created with a ``body_stream``,
684-
calling this method will raise :class:`RuntimeError`.
685-
686-
HTTP/2 does not have this limitation — see
687-
:meth:`Http2ClientStream.write_data`.
688-
689-
.. deprecated:: 0.x
690-
This method supersedes the C-level ``aws_http1_stream_write_chunk``
691-
API. Use ``write_data`` for all manual body writes on both HTTP/1.1
692-
and HTTP/2 streams.
698+
This method supersedes the deprecated ``write_chunk`` API.
699+
Use ``write_data()`` for all manual body writes on both HTTP/1.1
700+
and HTTP/2 streams.
693701
694702
Args:
695703
data_stream: Data to write. Wrapped in :class:`~awscrt.io.InputStream` if
@@ -698,18 +706,7 @@ def write_data(self,
698706
699707
Returns:
700708
concurrent.futures.Future: Completes with ``None`` on success.
701-
702-
Raises:
703-
RuntimeError: If the request has a ``body_stream`` set.
704709
'''
705-
if self._request and self._request.body_stream is not None:
706-
raise RuntimeError(
707-
"Cannot use write_data() on an HTTP/1.1 stream whose HttpRequest "
708-
"has a body_stream set. The H1 encoder is modal — it picks one "
709-
"body delivery path at init time and cannot switch mid-stream. "
710-
"Either remove the body_stream and use write_data() exclusively, "
711-
"or use the body_stream without calling write_data()."
712-
)
713710
return super().write_data(data_stream, end_stream)
714711

715712

@@ -781,21 +778,11 @@ def write_data(self,
781778
The stream must have been created with ``manual_write=True`` and
782779
:meth:`activate()` must have been called before using this method.
783780
784-
Unlike HTTP/1.1, HTTP/2 uses a queue-based write design
785-
(``outgoing_writes`` queue of ``aws_h2_stream_data_write`` structs).
786-
The body stream, if present, is simply the first item in the queue,
787-
and manual writes append additional items. The connection scheduler
788-
drains the queue and encodes DATA frames regardless of origin. This
789-
means HTTP/2 **can** combine a ``body_stream`` with subsequent
790-
``write_data()`` calls — the two compose naturally.
791-
792-
This is a key design difference from HTTP/1.1, whose encoder is
793-
*modal* (picks one body path at init) rather than *compositional*.
794-
795-
.. deprecated:: 0.x
796-
This method supersedes the C-level ``aws_http1_stream_write_chunk``
797-
API. Use ``write_data`` for all manual body writes on both HTTP/1.1
798-
and HTTP/2 streams.
781+
When both a body stream and manual writes are enabled, the body
782+
stream is sent first and the connection then waits asynchronously
783+
for subsequent ``write_data()`` calls. However, if the body stream
784+
has not signalled end-of-stream, the event loop will keep getting
785+
scheduled for requesting more data until it completes.
799786
800787
Args:
801788
data_stream: Data to write. Wrapped in :class:`~awscrt.io.InputStream` if

test/test_http_client.py

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,5 +961,129 @@ def test_h2_remote_end_stream_ordering(self):
961961
connection.close().result(self.timeout)
962962

963963

964+
@unittest.skipUnless(os.environ.get('AWS_TEST_LOCALHOST'), 'set env var to run test: AWS_TEST_LOCALHOST')
965+
class TestH1WriteData(LocalServerTestBase):
966+
"""HTTP/1.1 write_data() tests — mirrors Java WriteDataTest scenarios.
967+
Uses the existing LocalServerTestBase and PUT echo pattern."""
968+
timeout = 10
969+
970+
def _new_h1_tls_connection(self):
971+
"""Create HTTP/1.1 TLS connection to local server"""
972+
tls_ctx_opt = TlsContextOptions()
973+
tls_ctx_opt.verify_peer = False
974+
tls_ctx = ClientTlsContext(tls_ctx_opt)
975+
tls_conn_opt = tls_ctx.new_connection_options()
976+
tls_conn_opt.set_server_name(self.hostname)
977+
978+
event_loop_group = EventLoopGroup()
979+
host_resolver = DefaultHostResolver(event_loop_group)
980+
bootstrap = ClientBootstrap(event_loop_group, host_resolver)
981+
982+
connection = HttpClientConnection.new(
983+
host_name=self.hostname,
984+
port=self.port,
985+
bootstrap=bootstrap,
986+
tls_connection_options=tls_conn_opt,
987+
).result(self.timeout)
988+
self.assertEqual(connection.version, HttpVersion.Http1_1)
989+
return connection
990+
991+
def test_h1_write_data(self):
992+
"""H1 PUT with manual write — mirrors Java testHttp1WriteData"""
993+
self._start_server(secure=True)
994+
try:
995+
connection = self._new_h1_tls_connection()
996+
payload = b'hello from writeData h1'
997+
998+
request = HttpRequest('PUT', '/write_data_test')
999+
request.headers.add('host', self.hostname)
1000+
request.headers.add('Content-Length', str(len(payload)))
1001+
1002+
response = Response()
1003+
stream = connection.request(request, response.on_response, response.on_body, manual_write=True)
1004+
stream.activate()
1005+
1006+
stream.write_data(BytesIO(payload), end_stream=True).result(self.timeout)
1007+
status = stream.completion_future.result(self.timeout)
1008+
1009+
self.assertEqual(200, status)
1010+
self.assertEqual(payload, self.server.put_requests.get('/write_data_test'))
1011+
1012+
connection.close().result(self.timeout)
1013+
finally:
1014+
self._stop_server()
1015+
1016+
def test_h1_write_data_end_stream_only(self):
1017+
"""H1 PUT with zero-byte body — mirrors Java testHttp1WriteDataEndStreamOnly"""
1018+
self._start_server(secure=True)
1019+
try:
1020+
connection = self._new_h1_tls_connection()
1021+
1022+
request = HttpRequest('PUT', '/write_data_empty')
1023+
request.headers.add('host', self.hostname)
1024+
request.headers.add('Content-Length', '0')
1025+
1026+
response = Response()
1027+
stream = connection.request(request, response.on_response, response.on_body, manual_write=True)
1028+
stream.activate()
1029+
1030+
stream.write_data(None, end_stream=True).result(self.timeout)
1031+
status = stream.completion_future.result(self.timeout)
1032+
1033+
self.assertEqual(200, status)
1034+
self.assertEqual(b'', self.server.put_requests.get('/write_data_empty'))
1035+
1036+
connection.close().result(self.timeout)
1037+
finally:
1038+
self._stop_server()
1039+
1040+
def test_h1_write_data_multi_chunk(self):
1041+
"""H1 PUT with multiple write_data calls"""
1042+
self._start_server(secure=True)
1043+
try:
1044+
connection = self._new_h1_tls_connection()
1045+
chunks = [b'chunk1', b'chunk2', b'chunk3']
1046+
total = b''.join(chunks)
1047+
1048+
request = HttpRequest('PUT', '/write_data_multi')
1049+
request.headers.add('host', self.hostname)
1050+
request.headers.add('Content-Length', str(len(total)))
1051+
1052+
response = Response()
1053+
stream = connection.request(request, response.on_response, response.on_body, manual_write=True)
1054+
stream.activate()
1055+
1056+
for i, chunk in enumerate(chunks):
1057+
stream.write_data(BytesIO(chunk), end_stream=(i == len(chunks) - 1)).result(self.timeout)
1058+
1059+
status = stream.completion_future.result(self.timeout)
1060+
1061+
self.assertEqual(200, status)
1062+
self.assertEqual(total, self.server.put_requests.get('/write_data_multi'))
1063+
1064+
connection.close().result(self.timeout)
1065+
finally:
1066+
self._stop_server()
1067+
1068+
def test_h1_write_data_with_body_stream_raises(self):
1069+
"""H1 request() must raise ValueError if manual_write=True and body_stream is set."""
1070+
self._start_server(secure=True)
1071+
try:
1072+
connection = self._new_h1_tls_connection()
1073+
1074+
request = HttpRequest('PUT', '/write_data_guard', body_stream=BytesIO(b'body'))
1075+
request.headers.add('host', self.hostname)
1076+
request.headers.add('Content-Length', '4')
1077+
1078+
with self.assertRaises(ValueError) as ctx:
1079+
connection.request(request, manual_write=True)
1080+
1081+
self.assertIn('body_stream', str(ctx.exception))
1082+
1083+
connection.close().result(self.timeout)
1084+
finally:
1085+
self._stop_server()
1086+
1087+
9641088
if __name__ == '__main__':
9651089
unittest.main()

0 commit comments

Comments
 (0)