From 4ea1fd329b93e4263700a899f3fa832eb0d9d2a5 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 15 Apr 2024 09:50:51 -0700 Subject: [PATCH 01/12] Latest Submodules (#554) --- .gitignore | 80 +++++++++++++++++++++++++++++++++++++++++++++++- crt/aws-c-auth | 2 +- crt/aws-c-cal | 2 +- crt/aws-c-common | 2 +- crt/aws-c-io | 2 +- crt/aws-c-s3 | 2 +- crt/aws-lc | 2 +- 7 files changed, 85 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index e81f6b675..85e5eccff 100644 --- a/.gitignore +++ b/.gitignore @@ -187,7 +187,7 @@ ipython_config.py # pyenv # For a library or package, you might want to ignore these files since the code is # intended to run in multiple environments; otherwise, check them in: -# .python-version +.python-version # pipenv # According to pypa/pipenv#598, it is recommended to include Pipfile.lock in version control. @@ -261,6 +261,84 @@ cython_debug/ # option (not recommended) you can uncomment the following to ignore the entire idea folder. #.idea/ +# Covers JetBrains IDEs: IntelliJ, RubyMine, PhpStorm, AppCode, PyCharm, CLion, Android Studio, WebStorm and Rider +# Reference: https://intellij-support.jetbrains.com/hc/en-us/articles/206544839 + +# User-specific stuff +.idea/**/workspace.xml +.idea/**/tasks.xml +.idea/**/usage.statistics.xml +.idea/**/dictionaries +.idea/**/shelf + +# AWS User-specific +.idea/**/aws.xml + +# Generated files +.idea/**/contentModel.xml + +# Sensitive or high-churn files +.idea/**/dataSources/ +.idea/**/dataSources.ids +.idea/**/dataSources.local.xml +.idea/**/sqlDataSources.xml +.idea/**/dynamic.xml +.idea/**/uiDesigner.xml +.idea/**/dbnavigator.xml + +# Gradle +.idea/**/gradle.xml +.idea/**/libraries + +# Gradle and Maven with auto-import +# When using Gradle or Maven with auto-import, you should exclude module files, +# since they will be recreated, and may cause churn. Uncomment if using +# auto-import. +# .idea/artifacts +# .idea/compiler.xml +# .idea/jarRepositories.xml +# .idea/modules.xml +# .idea/*.iml +# .idea/modules +# *.iml +# *.ipr + +# CMake +cmake-build-*/ + +# Mongo Explorer plugin +.idea/**/mongoSettings.xml + +# File-based project format +*.iws + +# IntelliJ +out/ + +# mpeltonen/sbt-idea plugin +.idea_modules/ + +# JIRA plugin +atlassian-ide-plugin.xml + +# Cursive Clojure plugin +.idea/replstate.xml + +# SonarLint plugin +.idea/sonarlint/ + +# Crashlytics plugin (for Android Studio and IntelliJ) +com_crashlytics_export_strings.xml +crashlytics.properties +crashlytics-build.properties +fabric.properties + +# Editor-based Rest Client +.idea/httpRequests + +# Android studio 3.1+ serialized cache file +.idea/caches/build_file_checksums.ser + ### Python Patch ### # Poetry local configuration file - https://python-poetry.org/docs/configuration/#local-configuration poetry.toml diff --git a/crt/aws-c-auth b/crt/aws-c-auth index 0d2aa00ae..0de6b271b 160000 --- a/crt/aws-c-auth +++ b/crt/aws-c-auth @@ -1 +1 @@ -Subproject commit 0d2aa00ae70c699fcb14d0338c1b07a58b9eb24b +Subproject commit 0de6b271bdfb447853d1af0eced39faed7f746f3 diff --git a/crt/aws-c-cal b/crt/aws-c-cal index 56f0a79ce..314fc5558 160000 --- a/crt/aws-c-cal +++ b/crt/aws-c-cal @@ -1 +1 @@ -Subproject commit 56f0a79ceb10f2efcf92f525ace717f84d8c8a11 +Subproject commit 314fc555846ac7bf2cc68a117c99a6af26f7043e diff --git a/crt/aws-c-common b/crt/aws-c-common index fcadc0dd5..ae7b067d9 160000 --- a/crt/aws-c-common +++ b/crt/aws-c-common @@ -1 +1 @@ -Subproject commit fcadc0dd5d8a26134c8bbf08c58e30eff50d177b +Subproject commit ae7b067d9274d2d3faa1d3ae42d489a6986661f7 diff --git a/crt/aws-c-io b/crt/aws-c-io index 5afc94435..bf2d72230 160000 --- a/crt/aws-c-io +++ b/crt/aws-c-io @@ -1 +1 @@ -Subproject commit 5afc94435cc0c0b3dc33279dd62552ea15bbea0c +Subproject commit bf2d72230727f02eddb5b16c4e6c5ac5a3f203a7 diff --git a/crt/aws-c-s3 b/crt/aws-c-s3 index efeb625c7..3334843eb 160000 --- a/crt/aws-c-s3 +++ b/crt/aws-c-s3 @@ -1 +1 @@ -Subproject commit efeb625c7d8e6d789ee71c6863da3fff820c2dc2 +Subproject commit 3334843eb4e0e56c2565e481b2ef853d1cadde78 diff --git a/crt/aws-lc b/crt/aws-lc index 4e690737e..6284822a2 160000 --- a/crt/aws-lc +++ b/crt/aws-lc @@ -1 +1 @@ -Subproject commit 4e690737e0a386f8c5eb9a0a88becc7985b5d24e +Subproject commit 6284822a2e30d9a8497ba7e07bf3f506808912f2 From c11a5542141c1508d259a08e1c1d1db6a0b4c0a0 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 15 Apr 2024 14:13:12 -0700 Subject: [PATCH 02/12] Empty Commit to retrigger Release Pipeline (#557) From d76c3dacc94c1aa7dfc7346a77be78dc990b5171 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Mon, 6 May 2024 11:48:49 -0700 Subject: [PATCH 03/12] Pass CRT error, if it exists, into completion function (#563) --- .github/workflows/ci.yml | 2 +- .github/workflows/docs.yml | 2 +- awscrt/mqtt.py | 8 ++++++-- awscrt/mqtt5.py | 8 ++++++-- source/mqtt5_client.c | 8 ++++---- source/mqtt_client_connection.c | 8 ++++---- 6 files changed, 22 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 96b1f06c2..c2d00a14d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -272,7 +272,7 @@ jobs: submodules: true - name: Check docs run: | - python3 -m pip install sphinx + python3 -m pip install sphinx==7.2.6 python3 -m pip install --verbose . ./scripts/make-docs.py diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index a0ca31620..1a28f6464 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -20,7 +20,7 @@ jobs: - name: Update docs branch run: | - python3 -m pip install sphinx + python3 -m pip install sphinx==7.2.6 python3 -m pip install --verbose . ./scripts/make-docs.py diff --git a/awscrt/mqtt.py b/awscrt/mqtt.py index 5a96f5632..7529168ea 100644 --- a/awscrt/mqtt.py +++ b/awscrt/mqtt.py @@ -444,11 +444,15 @@ def _on_connection_resumed(self, return_code, session_present): def _ws_handshake_transform(self, http_request_binding, http_headers_binding, native_userdata): if self._ws_handshake_transform_cb is None: - _awscrt.mqtt_ws_handshake_transform_complete(None, native_userdata) + _awscrt.mqtt_ws_handshake_transform_complete(None, native_userdata, 0) return def _on_complete(f): - _awscrt.mqtt_ws_handshake_transform_complete(f.exception(), native_userdata) + error_code = 0 + hs_exception = f.exception() + if isinstance(hs_exception, awscrt.exceptions.AwsCrtError): + error_code = hs_exception.code + _awscrt.mqtt_ws_handshake_transform_complete(f.exception(), native_userdata, error_code) future = Future() future.add_done_callback(_on_complete) diff --git a/awscrt/mqtt5.py b/awscrt/mqtt5.py index 80dd0d002..4f8bff98d 100644 --- a/awscrt/mqtt5.py +++ b/awscrt/mqtt5.py @@ -1391,11 +1391,15 @@ def __init__(self, client_options: ClientOptions): def _ws_handshake_transform(self, http_request_binding, http_headers_binding, native_userdata): if self._ws_handshake_transform_cb is None: - _awscrt.mqtt5_ws_handshake_transform_complete(None, native_userdata) + _awscrt.mqtt5_ws_handshake_transform_complete(None, native_userdata, 0) return def _on_complete(f): - _awscrt.mqtt5_ws_handshake_transform_complete(f.exception(), native_userdata) + error_code = 0 + hs_exception = f.exception() + if isinstance(hs_exception, exceptions.AwsCrtError): + error_code = hs_exception.code + _awscrt.mqtt5_ws_handshake_transform_complete(f.exception(), native_userdata, error_code) future = Future() future.add_done_callback(_on_complete) diff --git a/source/mqtt5_client.c b/source/mqtt5_client.c index 92e2b1ea1..eddc5ab7f 100644 --- a/source/mqtt5_client.c +++ b/source/mqtt5_client.c @@ -711,13 +711,13 @@ PyObject *aws_py_mqtt5_ws_handshake_transform_complete(PyObject *self, PyObject PyObject *exception_py; PyObject *ws_transform_capsule; - if (!PyArg_ParseTuple(args, "OO", &exception_py, &ws_transform_capsule)) { + int error_code = AWS_ERROR_SUCCESS; + if (!PyArg_ParseTuple(args, "OOi", &exception_py, &ws_transform_capsule, &error_code)) { return NULL; } - int error_code = AWS_ERROR_SUCCESS; - if (exception_py != Py_None) { - /* TODO: Translate Python exception to aws error. In the meantime here's a catch-all. */ + if (exception_py != Py_None && error_code == AWS_ERROR_SUCCESS) { + /* Fallback code for if the error source was outside the CRT native implementation */ error_code = AWS_ERROR_HTTP_CALLBACK_FAILURE; } diff --git a/source/mqtt_client_connection.c b/source/mqtt_client_connection.c index 70da32835..9eb73a950 100644 --- a/source/mqtt_client_connection.c +++ b/source/mqtt_client_connection.c @@ -615,13 +615,13 @@ PyObject *aws_py_mqtt_ws_handshake_transform_complete(PyObject *self, PyObject * PyObject *exception_py; PyObject *ws_transform_capsule; - if (!PyArg_ParseTuple(args, "OO", &exception_py, &ws_transform_capsule)) { + int error_code = AWS_ERROR_SUCCESS; + if (!PyArg_ParseTuple(args, "OOi", &exception_py, &ws_transform_capsule, &error_code)) { return NULL; } - int error_code = AWS_ERROR_SUCCESS; - if (exception_py != Py_None) { - /* TODO: Translate Python exception to aws error. In the meantime here's a catch-all. */ + if (exception_py != Py_None && error_code == AWS_ERROR_SUCCESS) { + /* Fallback code for if the error source was outside the CRT native implementation */ error_code = AWS_ERROR_HTTP_CALLBACK_FAILURE; } From b5c797283c0208d947b1b788b260fc688e29290e Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Wed, 22 May 2024 10:00:18 -0700 Subject: [PATCH 04/12] Treat correlation data as binary not a nullable string (#560) Co-authored-by: Bret Ambrose --- awscrt/mqtt5.py | 29 ++++++-- source/mqtt5_client.c | 4 +- test/test_mqtt.py | 18 +++-- test/test_mqtt5.py | 167 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 201 insertions(+), 17 deletions(-) diff --git a/awscrt/mqtt5.py b/awscrt/mqtt5.py index 4f8bff98d..42bc68632 100644 --- a/awscrt/mqtt5.py +++ b/awscrt/mqtt5.py @@ -5,7 +5,7 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0. -from typing import Any, Callable +from typing import Any, Callable, Union import _awscrt from concurrent.futures import Future from enum import IntEnum @@ -1104,8 +1104,9 @@ class PublishPacket: message_expiry_interval_sec (int): Sent publishes - indicates the maximum amount of time allowed to elapse for message delivery before the server should instead delete the message (relative to a recipient). Received publishes - indicates the remaining amount of time (from the server's perspective) before the message would have been deleted relative to the subscribing client. If left None, indicates no expiration timeout. topic_alias (int): An integer value that is used to identify the Topic instead of using the Topic Name. On outbound publishes, this will only be used if the outbound topic aliasing behavior has been set to Manual. response_topic (str): Opaque topic string intended to assist with request/response implementations. Not internally meaningful to MQTT5 or this client. - correlation_data (Any): Opaque binary data used to correlate between publish messages, as a potential method for request-response implementation. Not internally meaningful to MQTT5. - subscription_identifiers (Sequence[int]): The subscription identifiers of all the subscriptions this message matched. + correlation_data (Optional[Union[bytes, str]]): Deprecated, use `correlation_data_bytes` instead. Opaque binary data used to correlate between publish messages, as a potential method for request-response implementation. Not internally meaningful to MQTT5. For incoming publishes, this will be a utf8 string (if correlation data exists and it's convertible to utf-8) or None (either it didn't exist, or did but wasn't convertible) + correlation_data_bytes (Optional[Union[bytes, str]]): Opaque binary data used to correlate between publish messages, as a potential method for request-response implementation. Not internally meaningful to MQTT5. For outbound publishes, this field takes priority over `correlation_data`. For incoming publishes, this will be binary data if correlation data is set, otherwise it will be None. + subscription_identifiers (Sequence[int]): The subscription identifiers of all the subscriptions this message matched. This field is ignored on outbound publishes (setting it is a protocol error). content_type (str): Property specifying the content type of the payload. Not internally meaningful to MQTT5. user_properties (Sequence[UserProperty]): List of MQTT5 user properties included with the packet. """ @@ -1117,7 +1118,9 @@ class PublishPacket: message_expiry_interval_sec: int = None topic_alias: int = None response_topic: str = None - correlation_data: Any = None # Unicode objects are converted to C strings using 'utf-8' encoding + correlation_data_bytes: 'Optional[Union[bytes, str]]' = None # binary data if correlation data exists on the packet + # Deprecated. Incoming publishes: a string if correlation data exists on the packet and is convertible to utf-8 + correlation_data: 'Optional[Union[bytes, str]]' = None subscription_identifiers: 'Sequence[int]' = None # ignore attempts to set but provide in received packets content_type: str = None user_properties: 'Sequence[UserProperty]' = None @@ -1447,8 +1450,18 @@ def _on_publish( if topic_alias_exists: publish_packet.topic_alias = topic_alias publish_packet.response_topic = response_topic - publish_packet.correlation_data = correlation_data - if publish_packet.subscription_identifiers is not None: + + # hacky workaround to maintain behavioral backwards compatibility with deprecated parameter + if correlation_data is not None: + # `correlation_data_bytes` always has the correlation data, as binary data + publish_packet.correlation_data_bytes = correlation_data + try: + # `correlation_data` contains the correlation data as a utf-8 string, if it can be converted + publish_packet.correlation_data = correlation_data.decode("utf-8") + except Exception: + pass + + if subscription_identifiers_tuples is not None: publish_packet.subscription_identifiers = [subscription_identifier for (subscription_identifier) in subscription_identifiers_tuples] publish_packet.content_type = content_type @@ -1768,7 +1781,7 @@ def __init__(self, client_options: ClientOptions): will.message_expiry_interval_sec, will.topic_alias, will.response_topic, - will.correlation_data, + will.correlation_data_bytes or will.correlation_data, will.content_type, will.user_properties, client_options.session_behavior, @@ -1865,7 +1878,7 @@ def puback(error_code, qos, reason_code, reason_string, user_properties_tuples): publish_packet.message_expiry_interval_sec, publish_packet.topic_alias, publish_packet.response_topic, - publish_packet.correlation_data, + publish_packet.correlation_data_bytes or publish_packet.correlation_data, publish_packet.content_type, publish_packet.user_properties, puback) diff --git a/source/mqtt5_client.c b/source/mqtt5_client.c index eddc5ab7f..f5f9b8eea 100644 --- a/source/mqtt5_client.c +++ b/source/mqtt5_client.c @@ -260,7 +260,7 @@ static void s_on_publish_received(const struct aws_mqtt5_packet_publish_view *pu result = PyObject_CallMethod( client->client_core, "_on_publish", - "(y#iOs#OiOIOHs#z#Os#O)", + "(y#iOs#OiOIOHs#y#Os#O)", /* y */ publish_packet->payload.ptr, /* # */ publish_packet->payload.len, /* i */ (int)publish_packet->qos, @@ -276,7 +276,7 @@ static void s_on_publish_received(const struct aws_mqtt5_packet_publish_view *pu /* H */ (unsigned short)(publish_packet->topic_alias ? *publish_packet->topic_alias : 0), /* s */ publish_packet->response_topic ? publish_packet->response_topic->ptr : NULL, /* # */ publish_packet->response_topic ? publish_packet->response_topic->len : 0, - /* z */ publish_packet->correlation_data ? publish_packet->correlation_data->ptr : NULL, + /* y */ publish_packet->correlation_data ? publish_packet->correlation_data->ptr : NULL, /* # */ publish_packet->correlation_data ? publish_packet->correlation_data->len : 0, /* O */ subscription_identifier_count > 0 ? subscription_identifier_list : Py_None, /* s */ publish_packet->content_type ? publish_packet->content_type->ptr : NULL, diff --git a/test/test_mqtt.py b/test/test_mqtt.py index 41d722e48..c76bad7fa 100644 --- a/test/test_mqtt.py +++ b/test/test_mqtt.py @@ -168,8 +168,8 @@ def test_will(self): host_name=test_input_endpoint, port=8883, will=Will(self.TEST_TOPIC, QoS.AT_LEAST_ONCE, self.TEST_MSG, False), - ping_timeout_ms=500, - keep_alive_secs=1 + ping_timeout_ms=10000, + keep_alive_secs=30 ) connection.connect().result(TIMEOUT) @@ -177,7 +177,9 @@ def test_will(self): client=client, client_id=create_client_id(), host_name=test_input_endpoint, - port=8883 + port=8883, + ping_timeout_ms=10000, + keep_alive_secs=30 ) subscriber.connect().result(TIMEOUT) @@ -193,16 +195,18 @@ def on_message(**kwargs): self.assertEqual(self.TEST_TOPIC, suback['topic']) self.assertIs(QoS.AT_LEAST_ONCE, suback['qos']) - # Disconnect the will client to send the will - time.sleep(1.1) # wait 1.1 seconds to ensure we can make another client ID connect + # wait a few seconds to ensure we can make another client ID connect (don't trigger IoT Core limit) + time.sleep(2) + + # Disconnect the will client to send the will by making another connection with the same client id disconnecter = Connection( client=client, client_id=will_client_id, host_name=test_input_endpoint, port=8883, will=Will(self.TEST_TOPIC, QoS.AT_LEAST_ONCE, self.TEST_MSG, False), - ping_timeout_ms=500, - keep_alive_secs=1 + ping_timeout_ms=10000, + keep_alive_secs=30 ) disconnecter.connect().result(TIMEOUT) diff --git a/test/test_mqtt5.py b/test/test_mqtt5.py index 6446285e6..143fd04cf 100644 --- a/test/test_mqtt5.py +++ b/test/test_mqtt5.py @@ -1230,6 +1230,98 @@ def test_operation_will(self): client2.stop() callbacks2.future_stopped.result(TIMEOUT) + def do_will_correlation_data_test(self, outbound_correlation_data_bytes, outbound_correlation_data, + expected_correlation_data_bytes, expected_correlation_data): + input_host_name = _get_env_variable("AWS_TEST_MQTT5_IOT_CORE_HOST") + input_cert = _get_env_variable("AWS_TEST_MQTT5_IOT_CORE_RSA_CERT") + input_key = _get_env_variable("AWS_TEST_MQTT5_IOT_CORE_RSA_KEY") + + client_id_publisher = create_client_id() + topic_filter = "test/MQTT5_Binding_Python_" + client_id_publisher + + payload = "TEST WILL" + payload_bytes = payload.encode("utf-8") + + will_packet = mqtt5.PublishPacket( + payload="TEST WILL", + qos=mqtt5.QoS.AT_LEAST_ONCE, + topic=topic_filter, + correlation_data_bytes=outbound_correlation_data_bytes, + correlation_data=outbound_correlation_data + ) + + tls_ctx_options = io.TlsContextOptions.create_client_with_mtls_from_path( + input_cert, + input_key + ) + + client_options1 = mqtt5.ClientOptions( + host_name=input_host_name, + port=8883 + ) + client_options1.connect_options = mqtt5.ConnectPacket(client_id=client_id_publisher, + will_delay_interval_sec=0, + will=will_packet) + client_options1.tls_ctx = io.ClientTlsContext(tls_ctx_options) + callbacks1 = Mqtt5TestCallbacks() + client1 = self._create_client(client_options=client_options1, callbacks=callbacks1) + client1.start() + callbacks1.future_connection_success.result(TIMEOUT) + + client_options2 = mqtt5.ClientOptions( + host_name=input_host_name, + port=8883 + ) + client_options2.connect_options = mqtt5.ConnectPacket(client_id=create_client_id()) + client_options2.tls_ctx = io.ClientTlsContext(tls_ctx_options) + callbacks2 = Mqtt5TestCallbacks() + client2 = self._create_client(client_options=client_options2, callbacks=callbacks2) + client2.start() + callbacks2.future_connection_success.result(TIMEOUT) + + subscriptions = [] + subscriptions.append(mqtt5.Subscription(topic_filter=topic_filter, qos=mqtt5.QoS.AT_LEAST_ONCE)) + subscribe_packet = mqtt5.SubscribePacket( + subscriptions=subscriptions) + subscribe_future = client2.subscribe(subscribe_packet=subscribe_packet) + suback_packet = subscribe_future.result(TIMEOUT) + self.assertIsInstance(suback_packet, mqtt5.SubackPacket) + + disconnect_packet = mqtt5.DisconnectPacket(reason_code=mqtt5.DisconnectReasonCode.DISCONNECT_WITH_WILL_MESSAGE) + client1.stop(disconnect_packet=disconnect_packet) + callbacks1.future_stopped.result(TIMEOUT) + + received_will = callbacks2.future_publish_received.result(TIMEOUT) + self.assertIsInstance(received_will, mqtt5.PublishPacket) + self.assertEqual(received_will.payload, payload_bytes) + self.assertEqual(received_will.correlation_data_bytes, expected_correlation_data_bytes) + self.assertEqual(received_will.correlation_data, expected_correlation_data) + + client2.stop() + callbacks2.future_stopped.result(TIMEOUT) + + def test_will_correlation_data_bytes_binary(self): + correlation_data = bytearray(os.urandom(64)) + self.do_will_correlation_data_test(correlation_data, None, correlation_data, None) + + def test_will_correlation_data_bytes_string(self): + correlation_data = "CorrelationData" + correlation_data_as_bytes = correlation_data.encode('utf-8') + self.do_correlation_data_test(correlation_data, None, correlation_data_as_bytes, correlation_data) + + def test_will_correlation_data_binary(self): + correlation_data = bytearray(os.urandom(64)) + self.do_correlation_data_test(None, correlation_data, correlation_data, None) + + def test_will_correlation_data_string(self): + correlation_data = "CorrelationData" + correlation_data_as_bytes = correlation_data.encode('utf-8') + self.do_correlation_data_test(None, correlation_data, correlation_data_as_bytes, correlation_data) + + def test_will_correlation_data_bytes_binary_precedence(self): + correlation_data = bytearray(os.urandom(64)) + self.do_correlation_data_test(correlation_data, "Ignored", correlation_data, None) + def test_operation_binary_publish(self): input_host_name = _get_env_variable("AWS_TEST_MQTT5_IOT_CORE_HOST") input_cert = _get_env_variable("AWS_TEST_MQTT5_IOT_CORE_RSA_CERT") @@ -1292,6 +1384,81 @@ def test_operation_binary_publish(self): client.stop() callbacks.future_stopped.result(TIMEOUT) + def do_correlation_data_test(self, outbound_correlation_data_bytes, outbound_correlation_data, + expected_correlation_data_bytes, expected_correlation_data): + input_host_name = _get_env_variable("AWS_TEST_MQTT5_IOT_CORE_HOST") + input_cert = _get_env_variable("AWS_TEST_MQTT5_IOT_CORE_RSA_CERT") + input_key = _get_env_variable("AWS_TEST_MQTT5_IOT_CORE_RSA_KEY") + + client_id = create_client_id() + topic_filter = "test/MQTT5_Binding_Python_" + client_id + payload = bytearray(os.urandom(256)) + + client_options = mqtt5.ClientOptions( + host_name=input_host_name, + port=8883 + ) + tls_ctx_options = io.TlsContextOptions.create_client_with_mtls_from_path( + input_cert, + input_key + ) + client_options.tls_ctx = io.ClientTlsContext(tls_ctx_options) + callbacks = Mqtt5TestCallbacks() + client = self._create_client(client_options=client_options, callbacks=callbacks) + client.start() + callbacks.future_connection_success.result(TIMEOUT) + + subscriptions = [] + subscriptions.append(mqtt5.Subscription(topic_filter=topic_filter, qos=mqtt5.QoS.AT_LEAST_ONCE)) + subscribe_packet = mqtt5.SubscribePacket( + subscriptions=subscriptions) + subscribe_future = client.subscribe(subscribe_packet=subscribe_packet) + suback_packet = subscribe_future.result(TIMEOUT) + self.assertIsInstance(suback_packet, mqtt5.SubackPacket) + + publish_packet = mqtt5.PublishPacket( + payload=payload, + topic=topic_filter, + qos=mqtt5.QoS.AT_LEAST_ONCE, + correlation_data_bytes=outbound_correlation_data_bytes, + correlation_data=outbound_correlation_data) + + publish_future = client.publish(publish_packet=publish_packet) + publish_completion_data = publish_future.result(TIMEOUT) + puback_packet = publish_completion_data.puback + self.assertIsInstance(puback_packet, mqtt5.PubackPacket) + + received_publish = callbacks.future_publish_received.result(TIMEOUT) + self.assertIsInstance(received_publish, mqtt5.PublishPacket) + self.assertEqual(received_publish.payload, payload) + self.assertEqual(received_publish.correlation_data_bytes, expected_correlation_data_bytes) + self.assertEqual(received_publish.correlation_data, expected_correlation_data) + + client.stop() + callbacks.future_stopped.result(TIMEOUT) + + def test_operation_publish_correlation_data_bytes_binary(self): + correlation_data = bytearray(os.urandom(64)) + self.do_correlation_data_test(correlation_data, None, correlation_data, None) + + def test_operation_publish_correlation_data_bytes_string(self): + correlation_data = "CorrelationData" + correlation_data_as_bytes = correlation_data.encode('utf-8') + self.do_correlation_data_test(correlation_data, None, correlation_data_as_bytes, correlation_data) + + def test_operation_publish_correlation_data_binary(self): + correlation_data = bytearray(os.urandom(64)) + self.do_correlation_data_test(None, correlation_data, correlation_data, None) + + def test_operation_publish_correlation_data_string(self): + correlation_data = "CorrelationData" + correlation_data_as_bytes = correlation_data.encode('utf-8') + self.do_correlation_data_test(None, correlation_data, correlation_data_as_bytes, correlation_data) + + def test_operation_publish_correlation_data_bytes_binary_precedence(self): + correlation_data = bytearray(os.urandom(64)) + self.do_correlation_data_test(correlation_data, "Ignored", correlation_data, None) + # ============================================================== # OPERATION ERROR TEST CASES # ============================================================== From 68afb702cc56e768b945dcdcc10816099e6d3514 Mon Sep 17 00:00:00 2001 From: Steve Kim <86316075+sbSteveK@users.noreply.github.com> Date: Wed, 22 May 2024 10:22:29 -0700 Subject: [PATCH 05/12] Bind connack_timeout_ms for mqtt5 (#548) Co-authored-by: Bret Ambrose --- awscrt/mqtt5.py | 12 +++++++----- source/mqtt5_client.c | 19 +++++++++++++++++-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/awscrt/mqtt5.py b/awscrt/mqtt5.py index 42bc68632..dba460ee5 100644 --- a/awscrt/mqtt5.py +++ b/awscrt/mqtt5.py @@ -955,7 +955,8 @@ class ConnackPacket: Args: session_present (bool): True if the client rejoined an existing session on the server, false otherwise. - reason_code (ConnectReasonCode): Indicates either success or the reason for failure for the connection attempt. session_expiry_interval_sec (int): A time interval, in seconds, that the server will persist this connection's MQTT session state for. If present, this value overrides any session expiry specified in the preceding CONNECT packet. + reason_code (ConnectReasonCode): Indicates either success or the reason for failure for the connection attempt. + session_expiry_interval_sec (int): A time interval, in seconds, that the server will persist this connection's MQTT session state for. If present, this value overrides any session expiry specified in the preceding CONNECT packet. receive_maximum (int): The maximum amount of in-flight QoS 1 or 2 messages that the server is willing to handle at once. If omitted or None, the limit is based on the valid MQTT packet id space (65535). maximum_qos (QoS): The maximum message delivery quality of service that the server will allow on this connection. retain_available (bool): Indicates whether the server supports retained messages. If None, retained messages are supported. @@ -1236,10 +1237,10 @@ class OperationStatisticsData: """Dataclass containing some simple statistics about the current state of the client's queue of operations Args: - incomplete_operation_count (int): total number of operations submitted to the client that have not yet been completed. Unacked operations are a subset of this. - incomplete_operation_size (int): total packet size of operations submitted to the client that have not yet been completed. Unacked operations are a subset of this. - unacked_operation_count (int): total number of operations that have been sent to the server and are waiting for a corresponding ACK before they can be completed. - unacked_operation_size (int): total packet size of operations that have been sent to the server and are waiting for a corresponding ACK before they can be completed. + incomplete_operation_count (int): Total number of operations submitted to the client that have not yet been completed. Unacked operations are a subset of this. + incomplete_operation_size (int): Total packet size of operations submitted to the client that have not yet been completed. Unacked operations are a subset of this. + unacked_operation_count (int): Total number of operations that have been sent to the server and are waiting for a corresponding ACK before they can be completed. + unacked_operation_size (int): Total packet size of operations that have been sent to the server and are waiting for a corresponding ACK before they can be completed. """ incomplete_operation_count: int = 0 incomplete_operation_size: int = 0 @@ -1792,6 +1793,7 @@ def __init__(self, client_options: ClientOptions): client_options.max_reconnect_delay_ms, client_options.min_connected_time_to_reset_reconnect_delay_ms, client_options.ping_timeout_ms, + client_options.connack_timeout_ms, client_options.ack_timeout_sec, client_options.topic_aliasing_options, websocket_is_none, diff --git a/source/mqtt5_client.c b/source/mqtt5_client.c index f5f9b8eea..bcedffaf4 100644 --- a/source/mqtt5_client.c +++ b/source/mqtt5_client.c @@ -57,6 +57,7 @@ static const char *AWS_PYOBJECT_KEY_MAX_RECONNECT_DELAY_MS = "max_reconnect_dela static const char *AWS_PYOBJECT_KEY_MIN_CONNECTED_TIME_TO_RESET_RECONNECT_DELAY_MS = "min_connected_time_to_reset_reconnect_delay_ms"; static const char *AWS_PYOBJECT_KEY_PING_TIMEOUT_MS = "ping_timeout_ms"; +static const char *AWS_PYOBJECT_KEY_CONNACK_TIMEOUT_MS = "connack_timeout_ms"; static const char *AWS_PYOBJECT_KEY_ACK_TIMEOUT_SECONDS = "ack_timeout_seconds"; #define KEEP_ALIVE_INTERVAL_SECONDS 1200 @@ -815,7 +816,7 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { /* Connect Options */ struct aws_byte_cursor client_id; /* optional */ - PyObject *keep_alive_interval_sec_py; /* uint16_t */ + PyObject *keep_alive_interval_sec_py; /* optional uint16_t */ struct aws_byte_cursor username; /* optional */ struct aws_byte_cursor password; /* optional */ PyObject *session_expiry_interval_sec_py; /* optional uint32_t */ @@ -848,6 +849,7 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { PyObject *max_reconnect_delay_ms_py; /* optional uint64_t */ PyObject *min_connected_time_to_reset_reconnect_delay_ms_py; /* optional uint64_t */ PyObject *ping_timeout_ms_py; /* optional uint32_t */ + PyObject *connack_timeout_ms_py; /* optional uint32_t */ PyObject *ack_timeout_seconds_py; /* optional uint32_t */ PyObject *topic_aliasing_options_py; /* optional TopicAliasingOptions */ /* Callbacks */ @@ -856,7 +858,7 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { if (!PyArg_ParseTuple( args, - "Os#IOOOOz#Oz#z#OOOOOOOOOz*Oz#OOOz#z*z#OOOOOOOOOOOOO", + "Os#IOOOOz#Oz#z#OOOOOOOOOz*Oz#OOOz#z*z#OOOOOOOOOOOOOO", /* O */ &self_py, /* s */ &host_name.ptr, /* # */ &host_name.len, @@ -906,6 +908,7 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { /* O */ &max_reconnect_delay_ms_py, /* O */ &min_connected_time_to_reset_reconnect_delay_ms_py, /* O */ &ping_timeout_ms_py, + /* O */ &connack_timeout_ms_py, /* O */ &ack_timeout_seconds_py, /* O */ &topic_aliasing_options_py, @@ -1079,6 +1082,18 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { goto done; } + uint32_t connack_timeout_ms_tmp = 0; + if (PyObject_GetAsOptionalUint32( + connack_timeout_ms_py, + AWS_PYOBJECT_KEY_CLIENT_OPTIONS, + AWS_PYOBJECT_KEY_CONNACK_TIMEOUT_MS, + &connack_timeout_ms_tmp)) { + client_options.connack_timeout_ms = connack_timeout_ms_tmp; + } + if (PyErr_Occurred()) { + goto done; + } + uint32_t ack_timeout_seconds_tmp = 0; if (PyObject_GetAsOptionalUint32( ack_timeout_seconds_py, From 23efdd760d4e8d02618f055dfe5fb40ddde52d41 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Wed, 22 May 2024 11:10:28 -0700 Subject: [PATCH 06/12] Typing fixes (#566) Co-authored-by: Bret Ambrose --- awscrt/mqtt5.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awscrt/mqtt5.py b/awscrt/mqtt5.py index dba460ee5..7c5e4f31f 100644 --- a/awscrt/mqtt5.py +++ b/awscrt/mqtt5.py @@ -1084,12 +1084,12 @@ class UnsubackPacket: Args: reason_string (str): Additional diagnostic information about the result of the UNSUBSCRIBE attempt. user_properties (Sequence[UserProperty]): List of MQTT5 user properties included with the packet. - reason_codes (Sequence[DisconnectReasonCode]): A list of reason codes indicating the result of unsubscribing from each individual topic filter entry in the associated UNSUBSCRIBE packet. + reason_codes (Sequence[UnsubackReasonCode]): A list of reason codes indicating the result of unsubscribing from each individual topic filter entry in the associated UNSUBSCRIBE packet. """ reason_string: str = None user_properties: 'Sequence[UserProperty]' = None - reason_codes: 'Sequence[DisconnectReasonCode]' = None + reason_codes: 'Sequence[UnsubackReasonCode]' = None @dataclass From af29c1580726bb3eb9792d145eb631dbff508f42 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Tue, 4 Jun 2024 20:04:43 -0700 Subject: [PATCH 07/12] Fix bug where last few bytes on socket go unread (#567) **Issue:** awscrt.s3.S3Client is occasionally seeing errors when the server sends an HTTP response with a `Connection: close` header (meaning it intends to close the connection after the response is sent). The server is sending the full response, then immediately hanging up. But the last few bytes of the response never make it to the HTTP client. **Description of changes:** Update submodules, bringing in this fix: https://github.com/awslabs/aws-c-io/pull/642 ``` aws-c-auth v0.7.17 -> v0.7.22 aws-c-cal v0.6.11 -> v0.6.15 aws-c-common v0.9.15 -> v0.9.20 aws-c-http v0.8.1 -> v0.8.2 aws-c-io v0.14.7 -> v0.14.9 aws-c-mqtt v0.10.3 -> v0.10.4 aws-c-s3 v0.5.7 -> v0.5.10 aws-c-sdkutils v0.1.15 -> v0.1.16 s2n v1.4.11 -> v1.4.15 ``` --- crt/aws-c-auth | 2 +- crt/aws-c-cal | 2 +- crt/aws-c-common | 2 +- crt/aws-c-http | 2 +- crt/aws-c-io | 2 +- crt/aws-c-mqtt | 2 +- crt/aws-c-s3 | 2 +- crt/aws-c-sdkutils | 2 +- crt/s2n | 2 +- test/test_http_client.py | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crt/aws-c-auth b/crt/aws-c-auth index 0de6b271b..53a31bacf 160000 --- a/crt/aws-c-auth +++ b/crt/aws-c-auth @@ -1 +1 @@ -Subproject commit 0de6b271bdfb447853d1af0eced39faed7f746f3 +Subproject commit 53a31bacf2918e848e00b052d2e25cba0be069d9 diff --git a/crt/aws-c-cal b/crt/aws-c-cal index 314fc5558..96c47e339 160000 --- a/crt/aws-c-cal +++ b/crt/aws-c-cal @@ -1 +1 @@ -Subproject commit 314fc555846ac7bf2cc68a117c99a6af26f7043e +Subproject commit 96c47e339d030d1fa4eaca201be948bc4442510d diff --git a/crt/aws-c-common b/crt/aws-c-common index ae7b067d9..06cf4d82a 160000 --- a/crt/aws-c-common +++ b/crt/aws-c-common @@ -1 +1 @@ -Subproject commit ae7b067d9274d2d3faa1d3ae42d489a6986661f7 +Subproject commit 06cf4d82adad52723305dc2aeb06bb6439b5df7f diff --git a/crt/aws-c-http b/crt/aws-c-http index 98ec73ad0..d83f8d701 160000 --- a/crt/aws-c-http +++ b/crt/aws-c-http @@ -1 +1 @@ -Subproject commit 98ec73ad0c18b78ba08d40b4e60d97abf794f24d +Subproject commit d83f8d70143ddce5ab4e479175fbd44ba994211b diff --git a/crt/aws-c-io b/crt/aws-c-io index bf2d72230..878b4fa02 160000 --- a/crt/aws-c-io +++ b/crt/aws-c-io @@ -1 +1 @@ -Subproject commit bf2d72230727f02eddb5b16c4e6c5ac5a3f203a7 +Subproject commit 878b4fa027bda4041493f06e0562d5e98bb3deb8 diff --git a/crt/aws-c-mqtt b/crt/aws-c-mqtt index 74da9cadf..ed7bbd68c 160000 --- a/crt/aws-c-mqtt +++ b/crt/aws-c-mqtt @@ -1 +1 @@ -Subproject commit 74da9cadfa9dfd2179479fdc445617f5da3261ba +Subproject commit ed7bbd68c03d7022c915a2924740ab7992ad2311 diff --git a/crt/aws-c-s3 b/crt/aws-c-s3 index 3334843eb..6588f9a71 160000 --- a/crt/aws-c-s3 +++ b/crt/aws-c-s3 @@ -1 +1 @@ -Subproject commit 3334843eb4e0e56c2565e481b2ef853d1cadde78 +Subproject commit 6588f9a714ee7a8be1bddd63ea5ea1ea224d00b4 diff --git a/crt/aws-c-sdkutils b/crt/aws-c-sdkutils index 638fdd654..8c7af71f9 160000 --- a/crt/aws-c-sdkutils +++ b/crt/aws-c-sdkutils @@ -1 +1 @@ -Subproject commit 638fdd6548df85c599f82db7ea70fd9e44917ef5 +Subproject commit 8c7af71f91ed5b9d2a043d51f120495f43723f80 diff --git a/crt/s2n b/crt/s2n index 171c96a23..6d92b46d3 160000 --- a/crt/s2n +++ b/crt/s2n @@ -1 +1 @@ -Subproject commit 171c96a232eb2bf45415340378b55b3bb6dd29cd +Subproject commit 6d92b46d309a8f12fa08ad289020d9a41c925a66 diff --git a/test/test_http_client.py b/test/test_http_client.py index b9f413a21..28fbe4394 100644 --- a/test/test_http_client.py +++ b/test/test_http_client.py @@ -57,7 +57,7 @@ def _start_server(self, secure, http_1_0=False): self.server = HTTPServer((self.hostname, 0), TestRequestHandler) if secure: - context = ssl.SSLContext() + context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) context.load_cert_chain(certfile='test/resources/unittest.crt', keyfile="test/resources/unittest.key") self.server.socket = context.wrap_socket(self.server.socket, server_side=True) self.port = self.server.server_address[1] From 4a4fd221e3e7ff9f98099256a8a122a98c1dc731 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Wed, 5 Jun 2024 16:07:40 -0700 Subject: [PATCH 08/12] fix docs.yml (#568) **Issue:** the docs.yml job that runs on mainline has been broken for a while **Description of changes:** - Put the required sphinx version in 1 place (requirements-dev.txt) - sphinx 7.3+ breaks our docs, not sure why, don't have time to look into it... - Update docs jobs to run on latest ubuntu22.04 - sphinx requires python3.9+ now, but ubuntu20.04 has python3.8. I tried ubuntu24.04 but it complained about virtual environments and I don't have for further investigation --- .github/workflows/ci.yml | 2 +- .github/workflows/docs.yml | 4 ++-- requirements-dev.txt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2d00a14d..88535218b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -272,7 +272,7 @@ jobs: submodules: true - name: Check docs run: | - python3 -m pip install sphinx==7.2.6 + python3 -m pip install -r requirements-dev.txt python3 -m pip install --verbose . ./scripts/make-docs.py diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 1a28f6464..b32b191fa 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -9,7 +9,7 @@ on: jobs: update-docs-branch: - runs-on: ubuntu-20.04 # latest + runs-on: ubuntu-22.04 # latest permissions: contents: write # allow push steps: @@ -20,7 +20,7 @@ jobs: - name: Update docs branch run: | - python3 -m pip install sphinx==7.2.6 + python3 -m pip install -r requirements-dev.txt python3 -m pip install --verbose . ./scripts/make-docs.py diff --git a/requirements-dev.txt b/requirements-dev.txt index 84715e477..469d13c6a 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,5 +1,5 @@ autopep8 # for code formatting setuptools # for installing from source -sphinx # for building docs +sphinx>=7.2.6,<7.3; python_version >= '3.9' # for building docs websockets # for tests wheel # for building wheels From c9c84bc61a0bfe21ddb4bde208a10d325df5eee9 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Fri, 7 Jun 2024 12:53:32 -0700 Subject: [PATCH 09/12] Address issues with latest AWS-LC and OpenBSD (#569) **Issue:** The latest AWS-LC was crashing on OpenBSD 7.4, when running test `test.test_http_client.TestClient.test_connect_pq_tlsv1_0_2021_05` **Investigation:** AWS-LC added [OpenBSD 7.4 and 7.5 Support](https://github.com/aws/aws-lc/pull/1437) in [v1.26.0](https://github.com/aws/aws-lc/releases/tag/v1.26.0). [Ironically](https://www.youtube.com/watch?v=Jne9t8sHpUc), these changes broke our existing OpenBSD 7.4 CI. My understanding is: "support OpenBSD" means "support fancy assembly math, instead of using vanilla C code math" on OpenBSD. This fancy assembly math currently reads from the .text section of the library, which is forbidden if a library is linked with the `--execute-only` flag, which OpenBSD 7.4+ uses by default. **Description of changes:** - Update to AWS-LC v1.24.0 -> v1.28.0 - Set '-Wl,--no-execute-only' flag when building for OpenBSD and using AWS-LC - Add OpenBSD 7.4 and 7.5 to CI (OpenBSD supports its two most recent releases) --- .github/workflows/ci.yml | 11 +++++++++-- crt/aws-lc | 2 +- setup.py | 16 +++++++++++----- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 88535218b..2b1ef67ee 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -192,21 +192,28 @@ jobs: openbsd: runs-on: ubuntu-22.04 # latest + strategy: + fail-fast: false + matrix: + # OpenBSD only supports the two most recent releases + version: ['7.4', '7.5'] steps: # Cannot use builder to checkout as OpenBSD doesn't ship git in the base install - uses: actions/checkout@v3 with: submodules: true - name: Build ${{ env.PACKAGE_NAME }} + consumers - uses: cross-platform-actions/action@v0.23.0 + uses: cross-platform-actions/action@v0.24.0 with: operating_system: openbsd - version: '7.4' + version: ${{ matrix.version }} cpu_count: 4 shell: bash environment_variables: AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY AWS_DEFAULT_REGION AWS_REGION run: | sudo pkg_add awscli py3-pip py3-urllib3 + python3 -m venv .venv + source .venv/bin/activate python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz', 'builder')" chmod a+x builder ./builder build -p ${{ env.PACKAGE_NAME }} diff --git a/crt/aws-lc b/crt/aws-lc index 6284822a2..92bf53276 160000 --- a/crt/aws-lc +++ b/crt/aws-lc @@ -1 +1 @@ -Subproject commit 6284822a2e30d9a8497ba7e07bf3f506808912f2 +Subproject commit 92bf53276029a71f01303e5adb1c5dbc379f1150 diff --git a/setup.py b/setup.py index df0ff039e..43b053b4d 100644 --- a/setup.py +++ b/setup.py @@ -334,17 +334,23 @@ def awscrt_ext(): if using_system_libcrypto(): libraries += ['crypto'] + else: + # hide the symbols from libcrypto.a + # this prevents weird crashes if an application also ends up using + # libcrypto.so from the system's OpenSSL installation. + extra_link_args += ['-Wl,--exclude-libs,libcrypto.a'] + + # OpenBSD 7.4+ defaults to linking with --execute-only, which is bad for AWS-LC. + # See: https://github.com/aws/aws-lc/blob/4b07805bddc55f68e5ce8c42f215da51c7a4e099/CMakeLists.txt#L44-L53 + # (If AWS-LC's CMakeLists.txt removes these lines in the future, we can remove this hack here as well) + if sys.platform.startswith('openbsd'): + extra_link_args += ['-Wl,--no-execute-only'] # FreeBSD doesn't have execinfo as a part of libc like other Unix variant. # Passing linker flag to link execinfo properly if sys.platform.startswith('freebsd'): extra_link_args += ['-lexecinfo'] - # hide the symbols from libcrypto.a - # this prevents weird crashes if an application also ends up using - # libcrypto.so from the system's OpenSSL installation. - extra_link_args += ['-Wl,--exclude-libs,libcrypto.a'] - # python usually adds -pthread automatically, but we've observed # rare cases where that didn't happen, so let's be explicit. extra_link_args += ['-pthread'] From e88cce2674ab8cc39ed2104d10bdb90cb2f30b77 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Wed, 12 Jun 2024 09:54:48 -0700 Subject: [PATCH 10/12] latest submodules (#570) ``` aws-c-common v0.9.20 -> v0.9.21 aws-lc v1.28.0 -> v1.29.0 s2n v1.4.15 -> v1.4.16 ``` --- crt/aws-c-common | 2 +- crt/aws-lc | 2 +- crt/s2n | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crt/aws-c-common b/crt/aws-c-common index 06cf4d82a..4f874cea5 160000 --- a/crt/aws-c-common +++ b/crt/aws-c-common @@ -1 +1 @@ -Subproject commit 06cf4d82adad52723305dc2aeb06bb6439b5df7f +Subproject commit 4f874cea50a70bc6ebcd85c6ce1c6c0016b5aff4 diff --git a/crt/aws-lc b/crt/aws-lc index 92bf53276..4e54dd836 160000 --- a/crt/aws-lc +++ b/crt/aws-lc @@ -1 +1 @@ -Subproject commit 92bf53276029a71f01303e5adb1c5dbc379f1150 +Subproject commit 4e54dd8363396f257d7a2317c48101e18170e6fb diff --git a/crt/s2n b/crt/s2n index 6d92b46d3..114ccab0f 160000 --- a/crt/s2n +++ b/crt/s2n @@ -1 +1 @@ -Subproject commit 6d92b46d309a8f12fa08ad289020d9a41c925a66 +Subproject commit 114ccab0ff2cde491203ac841837d0d39b767412 From a486d701e33b44fa31138d2a292ecf81b1ec4cf9 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Thu, 13 Jun 2024 15:56:11 -0700 Subject: [PATCH 11/12] clang-format 18 (#571) see: https://github.com/awslabs/aws-c-common/pull/1113 remove the `./scripts/format-c.py` script in favor of having the same `./format-check.py -i` script in the same place as every other repo --- .github/workflows/lint.yml | 10 ++++----- format-check.py | 46 ++++++++++++++++++++++++++++++++++++++ format-check.sh | 24 -------------------- guides/dev/README.md | 9 ++++---- scripts/format-all.py | 2 +- scripts/format-c.py | 16 ------------- source/module.c | 6 ++--- source/mqtt5_client.c | 8 +++++-- 8 files changed, 65 insertions(+), 56 deletions(-) create mode 100755 format-check.py delete mode 100755 format-check.sh delete mode 100755 scripts/format-c.py diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 2690c6bb8..927417f98 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -9,17 +9,15 @@ on: jobs: clang-format: - runs-on: ubuntu-20.04 # latest + runs-on: ubuntu-24.04 # latest steps: - name: Checkout Sources - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: clang-format lint - uses: DoozyX/clang-format-lint-action@v0.3.1 - with: - # List of extensions to check - extensions: c,h + run: | + ./format-check.py autopep8: runs-on: ubuntu-20.04 # latest diff --git a/format-check.py b/format-check.py new file mode 100755 index 000000000..f9c17bdbe --- /dev/null +++ b/format-check.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 +import argparse +import os +from pathlib import Path +import re +from subprocess import list2cmdline, run +from tempfile import NamedTemporaryFile + +CLANG_FORMAT_VERSION = '18.1.6' + +INCLUDE_REGEX = re.compile(r'^source/.*\.(c|h)$') +EXCLUDE_REGEX = re.compile(r'^$') + +arg_parser = argparse.ArgumentParser(description="Check with clang-format") +arg_parser.add_argument('-i', '--inplace-edit', action='store_true', + help="Edit files inplace") +args = arg_parser.parse_args() + +os.chdir(Path(__file__).parent) + +# create file containing list of all files to format +filepaths_file = NamedTemporaryFile(delete=False) +for dirpath, dirnames, filenames in os.walk('.'): + for filename in filenames: + # our regexes expect filepath to use forward slash + filepath = Path(dirpath, filename).as_posix() + if not INCLUDE_REGEX.match(filepath): + continue + if EXCLUDE_REGEX.match(filepath): + continue + + filepaths_file.write(f"{filepath}\n".encode()) +filepaths_file.close() + +# use pipx to run clang-format from PyPI +# this is a simple way to run the same clang-format version regardless of OS +cmd = ['pipx', 'run', f'clang-format=={CLANG_FORMAT_VERSION}', + f'--files={filepaths_file.name}'] +if args.inplace_edit: + cmd += ['-i'] +else: + cmd += ['--Werror', '--dry-run'] + +print(f"{Path.cwd()}$ {list2cmdline(cmd)}") +if run(cmd).returncode: + exit(1) diff --git a/format-check.sh b/format-check.sh deleted file mode 100755 index fce95eabc..000000000 --- a/format-check.sh +++ /dev/null @@ -1,24 +0,0 @@ -#!/usr/bin/env bash - -if [[ -z $CLANG_FORMAT ]] ; then - CLANG_FORMAT=clang-format -fi - -if NOT type $CLANG_FORMAT 2> /dev/null ; then - echo "No appropriate clang-format found." - exit 1 -fi - -FAIL=0 -SOURCE_FILES=`find source -type f \( -name '*.h' -o -name '*.c' \)` -for i in $SOURCE_FILES -do - $CLANG_FORMAT -output-replacements-xml $i | grep -c " /dev/null - if [ $? -ne 1 ] - then - echo "$i failed clang-format check." - FAIL=1 - fi -done - -exit $FAIL diff --git a/guides/dev/README.md b/guides/dev/README.md index 9fda4c7c0..39dc0189e 100644 --- a/guides/dev/README.md +++ b/guides/dev/README.md @@ -179,10 +179,11 @@ the code is formatted correctly. `autopep8` is used for python code. You installed this earlier via `requirements-dev.txt`. -For C code `clang-format` is used (specifically version 9). -To install this on Mac using homebrew, run: +For C code `clang-format` is used. You need to install an exact version (see `CLANG_FORMAT_VERSION=...` line at the top of [format-check.py](../../format-check.py)) via [pipx](https://github.com/pypa/pipx). Doing this on MacOS looks like: ```sh -(.venv) $ brew install llvm@9 +(.venv) $ brew install pipx +(.venv) $ pipx ensurepath +(.venv) $ pipx install clang-format== ``` Use helper scripts to automatically format your code (or configure your IDE to do it): @@ -194,7 +195,7 @@ Use helper scripts to automatically format your code (or configure your IDE to d (.venv) $ python3 scripts/format-python.py # just format C files -(.venv) $ python3 scripts/format-c.py +(.venv) $ python3 format-check.py -i ``` ## Using an IDE diff --git a/scripts/format-all.py b/scripts/format-all.py index a185d3669..ec816f028 100755 --- a/scripts/format-all.py +++ b/scripts/format-all.py @@ -3,5 +3,5 @@ utils.chdir_project_root() -utils.run('python3 scripts/format-c.py') +utils.run('python3 format-check.py -i') utils.run('python3 scripts/format-python.py') diff --git a/scripts/format-c.py b/scripts/format-c.py deleted file mode 100755 index d52a1c8e6..000000000 --- a/scripts/format-c.py +++ /dev/null @@ -1,16 +0,0 @@ -#!/usr/bin/env python3 -import utils -import glob - -FILE_PATTERNS = [ - 'source/**/*.h', - 'source/**/*.c', -] - -utils.chdir_project_root() - -files = [] -for pattern in FILE_PATTERNS: - files.extend(glob.glob(pattern, recursive=True)) - -utils.run(['clang-format', '-i', *files]) diff --git a/source/module.c b/source/module.c index 0648957e5..983054e79 100644 --- a/source/module.c +++ b/source/module.c @@ -364,7 +364,8 @@ PyObject *PyErr_AwsLastError(void) { } #define AWS_DEFINE_ERROR_INFO_CRT(CODE, STR) \ - [(CODE)-AWS_ERROR_ENUM_BEGIN_RANGE(AWS_CRT_PYTHON_PACKAGE_ID)] = AWS_DEFINE_ERROR_INFO(CODE, STR, "aws-crt-python") + [(CODE) - AWS_ERROR_ENUM_BEGIN_RANGE(AWS_CRT_PYTHON_PACKAGE_ID)] = \ + AWS_DEFINE_ERROR_INFO(CODE, STR, "aws-crt-python") /* clang-format off */ static struct aws_error_info s_errors[] = { @@ -655,8 +656,7 @@ static void s_install_crash_handler(void) { * Definitions ******************************************************************************/ -#define AWS_PY_METHOD_DEF(NAME, FLAGS) \ - { #NAME, aws_py_##NAME, (FLAGS), NULL } +#define AWS_PY_METHOD_DEF(NAME, FLAGS) {#NAME, aws_py_##NAME, (FLAGS), NULL} static PyMethodDef s_module_methods[] = { /* Common */ diff --git a/source/mqtt5_client.c b/source/mqtt5_client.c index bcedffaf4..243af6a0e 100644 --- a/source/mqtt5_client.c +++ b/source/mqtt5_client.c @@ -272,7 +272,9 @@ static void s_on_publish_received(const struct aws_mqtt5_packet_publish_view *pu /* i */ (int)(publish_packet->payload_format ? *publish_packet->payload_format : 0), /* O */ publish_packet->message_expiry_interval_seconds ? Py_True : Py_False, /* I */ - (unsigned int)(publish_packet->message_expiry_interval_seconds ? *publish_packet->message_expiry_interval_seconds : 0), + (unsigned int)(publish_packet->message_expiry_interval_seconds + ? *publish_packet->message_expiry_interval_seconds + : 0), /* O */ publish_packet->topic_alias ? Py_True : Py_False, /* H */ (unsigned short)(publish_packet->topic_alias ? *publish_packet->topic_alias : 0), /* s */ publish_packet->response_topic ? publish_packet->response_topic->ptr : NULL, @@ -544,7 +546,9 @@ static void s_lifecycle_event_disconnection( /* i */ (int)(disconnect ? disconnect->reason_code : 0), /* O */ (disconnect && disconnect->session_expiry_interval_seconds) ? Py_True : Py_False, /* I */ - (unsigned int)((disconnect && disconnect->session_expiry_interval_seconds) ? *disconnect->session_expiry_interval_seconds : 0), + (unsigned int)((disconnect && disconnect->session_expiry_interval_seconds) + ? *disconnect->session_expiry_interval_seconds + : 0), /* s */ (disconnect && disconnect->reason_string) ? disconnect->reason_string->ptr : NULL, /* # */ (disconnect && disconnect->reason_string) ? disconnect->reason_string->len : 0, /* O */ user_property_count > 0 ? user_properties_list : Py_None, From 1d5103eb5eb49f3d6d7f264b94ba488e91a3197e Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Wed, 19 Jun 2024 13:12:43 -0700 Subject: [PATCH 12/12] Always use virtual environments in CI, because PEP 668 (#573) **Issue:** CI is not working on some newer OSs (Ubuntu 24.04, OpenBSD 7.5) due to [PEP 668](https://peps.python.org/pep-0668/) which disallows global installations from pip. **Description of changes:** Always run CI in a virtual environment (using path `.venv-builder`) --- .builder/actions/aws_crt_python.py | 64 +++++++------------------- .github/workflows/ci.yml | 6 +-- codebuild/linux-integration-tests.yml | 5 +- codebuild/mqtt5-python-canary-test.yml | 5 +- 4 files changed, 22 insertions(+), 58 deletions(-) diff --git a/.builder/actions/aws_crt_python.py b/.builder/actions/aws_crt_python.py index 4fe87713f..866e721cf 100644 --- a/.builder/actions/aws_crt_python.py +++ b/.builder/actions/aws_crt_python.py @@ -1,67 +1,37 @@ import Builder import argparse -import os +from pathlib import Path import sys -# Fall back on using the "{python}" builder variable -PYTHON_DEFAULT = '{python}' - class AWSCrtPython(Builder.Action): - python = PYTHON_DEFAULT - - # Some CI containers have pip installed via "rpm" or non-Python methods, and this causes issues when - # we try to update pip via "python -m pip install --upgrade" because there are no RECORD files present. - # Therefore, we have to seek alternative ways with a last resort of installing with "--ignore-installed" - # if nothing else works AND the builder is running in GitHub actions. - # As of writing, this is primarily an issue with the AL2-x64 image. - def try_to_upgrade_pip(self, env): - did_upgrade = False - - if (self.python == '{python}'): - self.python = env.config["variables"]["python"] - - pip_result = env.shell.exec(self.python, '-m', 'pip', 'install', '--upgrade', 'pip', check=False) - if pip_result.returncode == 0: - did_upgrade = True - else: - print("Could not update pip via normal pip upgrade. Next trying via package manager...") - - if (did_upgrade == False): - try: - Builder.InstallPackages(['pip']).run(env) - did_upgrade = True - except Exception: - print("Could not update pip via package manager. Next resorting to forcing an ignore install...") - - if (did_upgrade == False): - # Only run in GitHub actions by checking for specific environment variable - # Source: https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables - if (os.getenv("GITHUB_ACTIONS") is not None): - pip_result = env.shell.exec( - self.python, '-m', 'pip', 'install', '--upgrade', - '--ignore-installed', 'pip', check=False) - if pip_result.returncode == 0: - did_upgrade = True - else: - print("Could not update pip via ignore install! Something is terribly wrong!") - sys.exit(12) - else: - print("Not on GitHub actions - skipping reinstalling Pip. Update/Install pip manually and rerun the builder") def run(self, env): # allow custom python to be used parser = argparse.ArgumentParser() parser.add_argument('--python') args = parser.parse_known_args(env.args.args)[0] - self.python = args.python if args.python else PYTHON_DEFAULT + if args.python: + self.python = args.python + else: + # Fall back on using the "{python}" builder variable + self.python = env.config['variables']['python'] + + # Create a virtual environment and use that. + # Otherwise, in places like ubuntu 24.04, PEP 668 stops + # you from globally installing/upgrading packages + venv_dirpath = Path.cwd() / '.venv-builder' + env.shell.exec(self.python, '-m', 'venv', str(venv_dirpath), check=True) + if sys.platform == 'win32': + self.python = str(venv_dirpath / 'Scripts/python') + else: + self.python = str(venv_dirpath / 'bin/python') # Enable S3 tests env.shell.setenv('AWS_TEST_S3', '1') actions = [ - # Upgrade Pip via a number of different methods - self.try_to_upgrade_pip, + [self.python, '-m', 'pip', 'install', '--upgrade', 'pip'], [self.python, '-m', 'pip', 'install', '--upgrade', '--requirement', 'requirements-dev.txt'], Builder.SetupCrossCICrtEnvironment(), [self.python, '-m', 'pip', 'install', '--verbose', '.'], diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2b1ef67ee..69216b9e9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ on: - 'docs' env: - BUILDER_VERSION: v0.9.56 + BUILDER_VERSION: v0.9.59 BUILDER_SOURCE: releases BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-crt-python @@ -158,7 +158,7 @@ jobs: - name: Assert libcrypto.so used run: | # assert it's linked against the system's libcrypto.so - AWSCRT_PATH=`python3 -c "import _awscrt; print(_awscrt.__file__)"` + AWSCRT_PATH=`aws-crt-python/.venv-builder/bin/python3 -c "import _awscrt; print(_awscrt.__file__)"` printf "AWSCRT_PATH: $AWSCRT_PATH\n" LINKED_AGAINST=`ldd $AWSCRT_PATH` @@ -212,8 +212,6 @@ jobs: environment_variables: AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY AWS_DEFAULT_REGION AWS_REGION run: | sudo pkg_add awscli py3-pip py3-urllib3 - python3 -m venv .venv - source .venv/bin/activate python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz', 'builder')" chmod a+x builder ./builder build -p ${{ env.PACKAGE_NAME }} diff --git a/codebuild/linux-integration-tests.yml b/codebuild/linux-integration-tests.yml index 22689b36e..251acaa50 100644 --- a/codebuild/linux-integration-tests.yml +++ b/codebuild/linux-integration-tests.yml @@ -4,7 +4,7 @@ version: 0.2 env: shell: bash variables: - BUILDER_VERSION: v0.9.44 + BUILDER_VERSION: v0.9.59 BUILDER_SOURCE: releases BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-crt-python @@ -13,7 +13,7 @@ phases: commands: - add-apt-repository ppa:ubuntu-toolchain-r/test - apt-get update -y - - apt-get install gcc-7 cmake ninja-build python3 python3-pip -y + - apt-get install gcc-7 cmake ninja-build python3 python3-pip python3-venv -y pre_build: commands: - export CC=gcc-7 @@ -27,4 +27,3 @@ phases: post_build: commands: - echo Build completed on `date` - diff --git a/codebuild/mqtt5-python-canary-test.yml b/codebuild/mqtt5-python-canary-test.yml index 07cdbaef5..d2a380086 100644 --- a/codebuild/mqtt5-python-canary-test.yml +++ b/codebuild/mqtt5-python-canary-test.yml @@ -10,9 +10,6 @@ env: CANARY_CLIENT_COUNT: 10 CANARY_LOG_FILE: 'canary_log.txt' CANARY_LOG_LEVEL: 'ERROR' - BUILDER_VERSION: v0.9.21 - BUILDER_SOURCE: releases - BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-crt-python CANARY_TEST_EXE: 'python -m unittest --failfast --verbose 2>&1 | tee /tmp/tests.log test.test_mqtt5_canary' CANARY_SERVER_ARN: Mqtt5MosquittoSever @@ -36,4 +33,4 @@ phases: - $CODEBUILD_SRC_DIR/codebuild/mqtt5-python-canary-test.sh post_build: commands: - - echo Build completed on `date` \ No newline at end of file + - echo Build completed on `date`