From 050ebca068c8e2f6c534c24e3f7f93af279eb6b8 Mon Sep 17 00:00:00 2001 From: Dane Walton Date: Thu, 4 Aug 2022 16:21:49 -0700 Subject: [PATCH] Add null manifest scenario (#2297) --- sdk/src/azure/iot/az_iot_adu_client.c | 155 ++++++++++++++------------ sdk/tests/iot/adu/test_az_iot_adu.c | 60 ++++++++++ 2 files changed, 144 insertions(+), 71 deletions(-) diff --git a/sdk/src/azure/iot/az_iot_adu_client.c b/sdk/src/azure/iot/az_iot_adu_client.c index 71a13c7918..dd4de913a6 100644 --- a/sdk/src/azure/iot/az_iot_adu_client.c +++ b/sdk/src/azure/iot/az_iot_adu_client.c @@ -471,28 +471,33 @@ AZ_NODISCARD az_result az_iot_adu_client_parse_service_properties( _az_RETURN_IF_FAILED(az_json_reader_next_token(ref_json_reader)); - _az_RETURN_IF_FAILED(az_json_token_get_string( - &ref_json_reader->token, - (char*)az_span_ptr(buffer), - az_span_size(buffer), - &update_manifest_length)); - - // TODO: find a way to get rid of az_json_token_get_string (which adds a \0 at the end!!!!!!) - // Preferably have a function that does not copy anything. - // TODO: optmize the memory usage for update_manifest: - // Here we are copying the entire update manifest [originally escaped] json into - // update_request->update_manifest. Later az_iot_adu_client_parse_update_manifest - // parses that json into a az_iot_adu_client_update_manifest structure, by simply - // mapping the values of update_request->update_manifest. Option 1: there seems to be no - // workaround for update_request->update_manifest for copying with - // az_json_token_get_string, since the original update manifest comes as an - // escaped json. What can be done is to make it temporary, and parse the - // update manifest within az_iot_adu_client_parse_service_request, saving only - // the update manifest values in the (then) provided buffer. - // Option 2: Have a function in azure SDK core that can parse an escaped json, allowing - // us to - // avoid copying the update manifest at all. - update_request->update_manifest = split_az_span(buffer, update_manifest_length, &buffer); + if (ref_json_reader->token.kind != AZ_JSON_TOKEN_NULL) + { + + _az_RETURN_IF_FAILED(az_json_token_get_string( + &ref_json_reader->token, + (char*)az_span_ptr(buffer), + az_span_size(buffer), + &update_manifest_length)); + + // TODO: find a way to get rid of az_json_token_get_string (which adds a \0 at the + // end!!!!!!) + // Preferably have a function that does not copy anything. + // TODO: optmize the memory usage for update_manifest: + // Here we are copying the entire update manifest [originally escaped] json into + // update_request->update_manifest. Later az_iot_adu_client_parse_update_manifest + // parses that json into a az_iot_adu_client_update_manifest structure, by simply + // mapping the values of update_request->update_manifest. Option 1: there seems to be + // no workaround for update_request->update_manifest for copying with + // az_json_token_get_string, since the original update manifest comes as an + // escaped json. What can be done is to make it temporary, and parse the + // update manifest within az_iot_adu_client_parse_service_request, saving + // only the update manifest values in the (then) provided buffer. + // Option 2: Have a function in azure SDK core that can parse an escaped json, + // allowing us to + // avoid copying the update manifest at all. + update_request->update_manifest = split_az_span(buffer, update_manifest_length, &buffer); + } } else if (az_json_token_is_text_equal( &ref_json_reader->token, @@ -500,79 +505,87 @@ AZ_NODISCARD az_result az_iot_adu_client_parse_service_properties( { _az_RETURN_IF_FAILED(az_json_reader_next_token(ref_json_reader)); - required_size = ref_json_reader->token.size + NULL_TERM_CHAR_SIZE; + if (ref_json_reader->token.kind != AZ_JSON_TOKEN_NULL) + { + + required_size = ref_json_reader->token.size + NULL_TERM_CHAR_SIZE; - _az_RETURN_IF_NOT_ENOUGH_SIZE(buffer, required_size); + _az_RETURN_IF_NOT_ENOUGH_SIZE(buffer, required_size); - update_request->update_manifest_signature = split_az_span(buffer, required_size, &buffer); + update_request->update_manifest_signature = split_az_span(buffer, required_size, &buffer); - _az_RETURN_IF_FAILED(az_json_token_get_string( - &ref_json_reader->token, - (char*)az_span_ptr(update_request->update_manifest_signature), - az_span_size(update_request->update_manifest_signature), - &out_length)); + _az_RETURN_IF_FAILED(az_json_token_get_string( + &ref_json_reader->token, + (char*)az_span_ptr(update_request->update_manifest_signature), + az_span_size(update_request->update_manifest_signature), + &out_length)); - // TODO: find a way to get rid of az_json_token_get_string (which adds a \0 at the end!!!!!!) - // Preferably have a function that does not copy anything. - update_request->update_manifest_signature - = az_span_slice(update_request->update_manifest_signature, 0, out_length); + // TODO: find a way to get rid of az_json_token_get_string (which adds a \0 at the + // end!!!!!!) + // Preferably have a function that does not copy anything. + update_request->update_manifest_signature + = az_span_slice(update_request->update_manifest_signature, 0, out_length); + } } else if (az_json_token_is_text_equal( &ref_json_reader->token, AZ_SPAN_FROM_STR(AZ_IOT_ADU_CLIENT_AGENT_PROPERTY_NAME_FILEURLS))) { _az_RETURN_IF_FAILED(az_json_reader_next_token(ref_json_reader)); - RETURN_IF_JSON_TOKEN_NOT_TYPE(ref_json_reader, AZ_JSON_TOKEN_BEGIN_OBJECT); - _az_RETURN_IF_FAILED(az_json_reader_next_token(ref_json_reader)); - - while (ref_json_reader->token.kind != AZ_JSON_TOKEN_END_OBJECT) + if (ref_json_reader->token.kind != AZ_JSON_TOKEN_NULL) { - RETURN_IF_JSON_TOKEN_NOT_TYPE(ref_json_reader, AZ_JSON_TOKEN_PROPERTY_NAME); + RETURN_IF_JSON_TOKEN_NOT_TYPE(ref_json_reader, AZ_JSON_TOKEN_BEGIN_OBJECT); + _az_RETURN_IF_FAILED(az_json_reader_next_token(ref_json_reader)); - required_size = ref_json_reader->token.size + NULL_TERM_CHAR_SIZE; + while (ref_json_reader->token.kind != AZ_JSON_TOKEN_END_OBJECT) + { + RETURN_IF_JSON_TOKEN_NOT_TYPE(ref_json_reader, AZ_JSON_TOKEN_PROPERTY_NAME); - _az_RETURN_IF_NOT_ENOUGH_SIZE(buffer, required_size); + required_size = ref_json_reader->token.size + NULL_TERM_CHAR_SIZE; - update_request->file_urls[update_request->file_urls_count].id - = split_az_span(buffer, required_size, &buffer); + _az_RETURN_IF_NOT_ENOUGH_SIZE(buffer, required_size); - _az_RETURN_IF_FAILED(az_json_token_get_string( - &ref_json_reader->token, - (char*)az_span_ptr(update_request->file_urls[update_request->file_urls_count].id), - az_span_size(update_request->file_urls[update_request->file_urls_count].id), - &out_length)); + update_request->file_urls[update_request->file_urls_count].id + = split_az_span(buffer, required_size, &buffer); - // TODO: find a way to get rid of az_json_token_get_string (which adds a \0 at the - // end!!!!!!) - // Preferably have a function that does not copy anything. - update_request->file_urls[update_request->file_urls_count].id = az_span_slice( - update_request->file_urls[update_request->file_urls_count].id, 0, out_length); + _az_RETURN_IF_FAILED(az_json_token_get_string( + &ref_json_reader->token, + (char*)az_span_ptr(update_request->file_urls[update_request->file_urls_count].id), + az_span_size(update_request->file_urls[update_request->file_urls_count].id), + &out_length)); - _az_RETURN_IF_FAILED(az_json_reader_next_token(ref_json_reader)); - RETURN_IF_JSON_TOKEN_NOT_TYPE(ref_json_reader, AZ_JSON_TOKEN_STRING); + // TODO: find a way to get rid of az_json_token_get_string (which adds a \0 at the + // end!!!!!!) + // Preferably have a function that does not copy anything. + update_request->file_urls[update_request->file_urls_count].id = az_span_slice( + update_request->file_urls[update_request->file_urls_count].id, 0, out_length); - required_size = ref_json_reader->token.size + NULL_TERM_CHAR_SIZE; + _az_RETURN_IF_FAILED(az_json_reader_next_token(ref_json_reader)); + RETURN_IF_JSON_TOKEN_NOT_TYPE(ref_json_reader, AZ_JSON_TOKEN_STRING); - _az_RETURN_IF_NOT_ENOUGH_SIZE(buffer, required_size); + required_size = ref_json_reader->token.size + NULL_TERM_CHAR_SIZE; - update_request->file_urls[update_request->file_urls_count].url - = split_az_span(buffer, required_size, &buffer); + _az_RETURN_IF_NOT_ENOUGH_SIZE(buffer, required_size); - _az_RETURN_IF_FAILED(az_json_token_get_string( - &ref_json_reader->token, - (char*)az_span_ptr(update_request->file_urls[update_request->file_urls_count].url), - az_span_size(update_request->file_urls[update_request->file_urls_count].url), - &out_length)); + update_request->file_urls[update_request->file_urls_count].url + = split_az_span(buffer, required_size, &buffer); - // TODO: find a way to get rid of az_json_token_get_string (which adds a \0 at the - // end!!!!!!) - // Preferably have a function that does not copy anything. - update_request->file_urls[update_request->file_urls_count].url = az_span_slice( - update_request->file_urls[update_request->file_urls_count].url, 0, out_length); + _az_RETURN_IF_FAILED(az_json_token_get_string( + &ref_json_reader->token, + (char*)az_span_ptr(update_request->file_urls[update_request->file_urls_count].url), + az_span_size(update_request->file_urls[update_request->file_urls_count].url), + &out_length)); + + // TODO: find a way to get rid of az_json_token_get_string (which adds a \0 at the + // end!!!!!!) + // Preferably have a function that does not copy anything. + update_request->file_urls[update_request->file_urls_count].url = az_span_slice( + update_request->file_urls[update_request->file_urls_count].url, 0, out_length); - update_request->file_urls_count++; + update_request->file_urls_count++; - _az_RETURN_IF_FAILED(az_json_reader_next_token(ref_json_reader)); + _az_RETURN_IF_FAILED(az_json_reader_next_token(ref_json_reader)); + } } } diff --git a/sdk/tests/iot/adu/test_az_iot_adu.c b/sdk/tests/iot/adu/test_az_iot_adu.c index 453319a6f8..1a7225dbeb 100644 --- a/sdk/tests/iot/adu/test_az_iot_adu.c +++ b/sdk/tests/iot/adu/test_az_iot_adu.c @@ -146,6 +146,9 @@ static uint8_t adu_request_payload_reverse_order[] "westus2/contoso-adu-instance--contoso-adu/67c8d2ef5148403391bed74f51a28597/" "iot-middleware-sample-adu-v1.1\"},\"workflow\":{\"action\":3,\"id\":\"51552a54-765e-419f-" "892a-c822549b6f38\"}}}"; +static uint8_t adu_request_manifest_no_deployment_null_manifest[] + = "{\"service\":{\"workflow\":{\"action\":255,\"id\":\"nodeployment\"},\"updateManifest\":null," + "\"updateManifestSignature\":null,\"fileUrls\":null},\"__t\":\"c\"}"; static uint8_t adu_request_manifest[] = "{\"manifestVersion\":\"4\",\"updateId\":{\"provider\":\"Contoso\",\"name\":\"Foobar\"," "\"version\":\"1.1\"},\"compatibility\":[{\"deviceManufacturer\":\"Contoso\",\"deviceModel\":" @@ -164,7 +167,9 @@ static uint8_t adu_request_manifest_reverse_order[] "\"Foobar\"}],\"updateId\":{\"provider\":\"Contoso\",\"name\":\"Foobar\",\"version\":\"1.1\"}" ",\"manifestVersion\":\"4\"}"; static uint32_t workflow_action = 3; +static uint32_t workflow_action_failed = 255; static uint8_t workflow_id[] = "51552a54-765e-419f-892a-c822549b6f38"; +static uint8_t workflow_id_nodeployment[] = "nodeployment"; static uint8_t manifest_version[] = "4"; static uint8_t update_id_provider[] = "Contoso"; static uint8_t update_id_name[] = "Foobar"; @@ -613,6 +618,59 @@ static void test_az_iot_adu_client_parse_service_properties_payload_reverse_orde assert_int_equal(az_span_size(request.file_urls[0].url), sizeof(file_url) - 1); } +static void test_az_iot_adu_client_parse_service_properties_payload_no_deployment_null_manifest( + void** state) +{ + (void)state; + + az_iot_adu_client adu_client; + az_json_reader reader; + az_iot_adu_client_update_request request; + az_span remainder; + + assert_int_equal(az_iot_adu_client_init(&adu_client, NULL), AZ_OK); + + assert_int_equal( + az_json_reader_init( + &reader, + az_span_create( + adu_request_manifest_no_deployment_null_manifest, + sizeof(adu_request_manifest_no_deployment_null_manifest) - 1), + NULL), + AZ_OK); + + // parse_service_properties requires that the reader be placed on the "service" prop name + assert_int_equal(az_json_reader_next_token(&reader), AZ_OK); + assert_int_equal(az_json_reader_next_token(&reader), AZ_OK); + + assert_int_equal( + az_iot_adu_client_parse_service_properties( + &adu_client, + &reader, + az_span_create(scratch_buffer, sizeof(scratch_buffer)), + &request, + &remainder), + AZ_OK); + + // Workflow + assert_int_equal(request.workflow.action, workflow_action_failed); + assert_memory_equal( + az_span_ptr(request.workflow.id), + workflow_id_nodeployment, + sizeof(workflow_id_nodeployment) - 1); + + // Update Manifest + assert_null(az_span_ptr(request.update_manifest)); + assert_int_equal(az_span_size(request.update_manifest), 0); + + // Signature + assert_null(az_span_ptr(request.update_manifest_signature)); + assert_int_equal(az_span_size(request.update_manifest_signature), 0); + + // File URLs + assert_int_equal(request.file_urls_count, 0); +} + static void test_az_iot_adu_client_parse_update_manifest_succeed(void** state) { (void)state; @@ -817,6 +875,8 @@ int test_az_iot_adu() cmocka_unit_test(test_az_iot_adu_client_get_service_properties_response_succeed), cmocka_unit_test(test_az_iot_adu_client_parse_service_properties_succeed), cmocka_unit_test(test_az_iot_adu_client_parse_service_properties_payload_reverse_order_succeed), + cmocka_unit_test( + test_az_iot_adu_client_parse_service_properties_payload_no_deployment_null_manifest), cmocka_unit_test(test_az_iot_adu_client_parse_update_manifest_succeed), cmocka_unit_test(test_az_iot_adu_client_parse_update_manifest_payload_reverse_order_succeed), };