diff --git a/CHANGELOG.md b/CHANGELOG.md index 3118ef19b..abf94c3e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Add `az_json_writer_append_json_text()` to support appending existing JSON with the JSON writer. - Add support for system properties for IoT Hub messages to `az_iot_common.h`. +- Add new HTTP result named `AZ_ERROR_HTTP_END_OF_HEADERS` to designate the end of the headers iterated over by `az_http_response_get_next_header()`. - Add new IoT result named `AZ_ERROR_IOT_NO_MORE_PROPERTIES` to designate the end of the properties iterated over by `az_iot_message_properties_next()`. ### Breaking Changes diff --git a/sdk/inc/azure/core/az_http.h b/sdk/inc/azure/core/az_http.h index daf71db4c..ac85f31ba 100644 --- a/sdk/inc/azure/core/az_http.h +++ b/sdk/inc/azure/core/az_http.h @@ -243,7 +243,9 @@ AZ_NODISCARD az_result az_http_response_get_status_line( * * @return An #az_result value indicating the result of the operation. * @retval #AZ_OK A header was returned. - * @retval #AZ_ERROR_ITEM_NOT_FOUND There are no more headers. + * @retval #AZ_ERROR_HTTP_END_OF_HEADERS There are no more headers within the HTTP response payload. + * @retval #AZ_ERROR_HTTP_CORRUPT_RESPONSE_HEADER The HTTP response contains an unexpected invalid + * character or is incomplete. * @retval #AZ_ERROR_HTTP_INVALID_STATE The #az_http_response instance is in an invalid state. * Consider calling #az_http_response_get_status_line() to reset its state. */ diff --git a/sdk/inc/azure/core/az_result.h b/sdk/inc/azure/core/az_result.h index a8c93f20c..c32f85e28 100644 --- a/sdk/inc/azure/core/az_result.h +++ b/sdk/inc/azure/core/az_result.h @@ -141,9 +141,12 @@ typedef enum /// Error while parsing HTTP response header. AZ_ERROR_HTTP_CORRUPT_RESPONSE_HEADER = _az_RESULT_MAKE_ERROR(_az_FACILITY_HTTP, 7), + /// There are no more headers within the HTTP response payload. + AZ_ERROR_HTTP_END_OF_HEADERS = _az_RESULT_MAKE_ERROR(_az_FACILITY_HTTP, 8), + // === HTTP Adapter error codes === /// Generic error in the HTTP transport adapter implementation. - AZ_ERROR_HTTP_ADAPTER = _az_RESULT_MAKE_ERROR(_az_FACILITY_HTTP, 8), + AZ_ERROR_HTTP_ADAPTER = _az_RESULT_MAKE_ERROR(_az_FACILITY_HTTP, 9), // === IoT error codes === /// The IoT topic is not matching the expected format. diff --git a/sdk/samples/storage/blobs/src/blobs_client_example.c b/sdk/samples/storage/blobs/src/blobs_client_example.c index d67a7639f..8ae9da717 100644 --- a/sdk/samples/storage/blobs/src/blobs_client_example.c +++ b/sdk/samples/storage/blobs/src/blobs_client_example.c @@ -127,7 +127,7 @@ int main() { az_pair header; az_result const header_get_result = az_http_response_get_next_header(&http_response, &header); - if (header_get_result == AZ_ERROR_ITEM_NOT_FOUND) + if (header_get_result == AZ_ERROR_HTTP_END_OF_HEADERS) { break; } diff --git a/sdk/src/azure/core/az_http_policy_logging.c b/sdk/src/azure/core/az_http_policy_logging.c index db9841448..acdf1901f 100644 --- a/sdk/src/azure/core/az_http_policy_logging.c +++ b/sdk/src/azure/core/az_http_policy_logging.c @@ -160,8 +160,9 @@ static az_result _az_http_policy_logging_append_http_response_msg( az_span new_line_tab_string = AZ_SPAN_FROM_STR("\n\t"); - for (az_pair header; - az_http_response_get_next_header(ref_response, &header) != AZ_ERROR_ITEM_NOT_FOUND;) + az_result result = AZ_OK; + az_pair header = { 0 }; + while (az_result_succeeded(result = az_http_response_get_next_header(ref_response, &header))) { int32_t required_length = az_span_size(new_line_tab_string) + az_span_size(header.key); if (az_span_size(header.value) > 0) @@ -181,6 +182,12 @@ static az_result _az_http_policy_logging_append_http_response_msg( } } + // Response payload was invalid or corrupted in some way. + if (result != AZ_ERROR_HTTP_END_OF_HEADERS) + { + return result; + } + az_span new_lines_string = AZ_SPAN_FROM_STR("\n\n"); az_span arrow_separator_string = AZ_SPAN_FROM_STR(" -> "); int32_t required_length = az_span_size(new_lines_string) + az_span_size(arrow_separator_string); diff --git a/sdk/src/azure/core/az_http_response.c b/sdk/src/azure/core/az_http_response.c index b297088ca..5ba935aa7 100644 --- a/sdk/src/azure/core/az_http_response.c +++ b/sdk/src/azure/core/az_http_response.c @@ -138,11 +138,11 @@ az_http_response_get_next_header(az_http_response* ref_response, az_pair* out_he az_span* reader = &ref_response->_internal.parser.remaining; { _az_http_response_kind const kind = ref_response->_internal.parser.next_kind; - // if reader is expecting to read body (all headers were read), return ITEM_NOT_FOUND so we - // know we reach end of headers + // if reader is expecting to read body (all headers were read), return + // AZ_ERROR_HTTP_END_OF_HEADERS so we know we reach end of headers if (kind == _az_HTTP_RESPONSE_KIND_BODY) { - return AZ_ERROR_ITEM_NOT_FOUND; + return AZ_ERROR_HTTP_END_OF_HEADERS; } // Can't read a header if status line was not previously called, // User needs to call az_http_response_status_line() which would reset parser and set kind to @@ -159,7 +159,7 @@ az_http_response_get_next_header(az_http_response* ref_response, az_pair* out_he { AZ_RETURN_IF_FAILED(_az_is_expected_span(reader, AZ_SPAN_FROM_STR("\r\n"))); ref_response->_internal.parser.next_kind = _az_HTTP_RESPONSE_KIND_BODY; - return AZ_ERROR_ITEM_NOT_FOUND; + return AZ_ERROR_HTTP_END_OF_HEADERS; } // https://tools.ietf.org/html/rfc7230#section-3.2 @@ -189,7 +189,7 @@ az_http_response_get_next_header(az_http_response* ref_response, az_pair* out_he } if (field_name_length == input_size) { - return AZ_ERROR_ITEM_NOT_FOUND; + return AZ_ERROR_HTTP_CORRUPT_RESPONSE_HEADER; } // form a header name. Reader is currently at char ':' @@ -215,7 +215,7 @@ az_http_response_get_next_header(az_http_response* ref_response, az_pair* out_he } if (ows_len == input_size) { - return AZ_ERROR_ITEM_NOT_FOUND; + return AZ_ERROR_HTTP_CORRUPT_RESPONSE_HEADER; } *reader = az_span_slice_to_end(*reader, ows_len); @@ -244,9 +244,9 @@ az_http_response_get_next_header(az_http_response* ref_response, az_pair* out_he { continue; // whitespace or tab is accepted. It can be any number after value (OWS) } - if (c <= ' ') + if (c < ' ') { - return AZ_ERROR_UNEXPECTED_CHAR; + return AZ_ERROR_HTTP_CORRUPT_RESPONSE_HEADER; } offset_value_end = offset; // increasing index only for valid chars, } diff --git a/sdk/tests/core/test_az_http.c b/sdk/tests/core/test_az_http.c index 3dc545311..e5b079bd0 100644 --- a/sdk/tests/core/test_az_http.c +++ b/sdk/tests/core/test_az_http.c @@ -463,7 +463,7 @@ static void test_http_response(void** state) { az_pair header = { 0 }; result = az_http_response_get_next_header(&response, &header); - assert_true(result == AZ_ERROR_ITEM_NOT_FOUND); + assert_true(result == AZ_ERROR_HTTP_END_OF_HEADERS); } // read a body { @@ -516,7 +516,13 @@ static void test_http_response(void** state) { az_pair header = { 0 }; result = az_http_response_get_next_header(&response, &header); - assert_true(result == AZ_ERROR_ITEM_NOT_FOUND); + assert_true(result == AZ_ERROR_HTTP_END_OF_HEADERS); + + // Subsequent reads do nothing. + result = az_http_response_get_next_header(&response, &header); + assert_true(result == AZ_ERROR_HTTP_END_OF_HEADERS); + result = az_http_response_get_next_header(&response, &header); + assert_true(result == AZ_ERROR_HTTP_END_OF_HEADERS); } // read a body { @@ -685,7 +691,7 @@ static void test_http_response_header_validation(void** state) az_http_response_status_line status_line = { 0 }; assert_return_code(az_http_response_get_status_line(&response, &status_line), AZ_OK); for (az_pair header; - az_http_response_get_next_header(&response, &header) != AZ_ERROR_ITEM_NOT_FOUND;) + az_http_response_get_next_header(&response, &header) != AZ_ERROR_HTTP_END_OF_HEADERS;) { // all valid headers assert_true(az_span_ptr(header.key) != NULL); @@ -709,6 +715,58 @@ static void test_http_response_header_validation_fail(void** state) "KKKKKJJJJJIIIIIHHHHHGGGGGFFFFFEEEEEDDDDDCCCCCBBBBBAAAAA")), AZ_OK); + // Can't read the header first, before the status line. + az_pair header = { 0 }; + assert_int_equal( + az_http_response_get_next_header(&response, &header), AZ_ERROR_HTTP_INVALID_STATE); + + az_http_response_status_line status_line = { 0 }; + assert_return_code(az_http_response_get_status_line(&response, &status_line), AZ_OK); + + az_result fail_header_result = az_http_response_get_next_header(&response, &header); + assert_true(AZ_ERROR_HTTP_CORRUPT_RESPONSE_HEADER == fail_header_result); + } + + // Invalid end of header. + { + az_http_response response = { 0 }; + assert_return_code( + az_http_response_init( + &response, + AZ_SPAN_FROM_STR("HTTP/1.1 404 Not Found\r\n" + "Header11")), + AZ_OK); + + az_http_response_status_line status_line = { 0 }; + assert_return_code(az_http_response_get_status_line(&response, &status_line), AZ_OK); + az_pair header = { 0 }; + az_result fail_header_result = az_http_response_get_next_header(&response, &header); + assert_true(AZ_ERROR_HTTP_CORRUPT_RESPONSE_HEADER == fail_header_result); + } + { + az_http_response response = { 0 }; + assert_return_code( + az_http_response_init( + &response, + AZ_SPAN_FROM_STR("HTTP/1.1 404 Not Found\r\n" + "Header11: ")), + AZ_OK); + + az_http_response_status_line status_line = { 0 }; + assert_return_code(az_http_response_get_status_line(&response, &status_line), AZ_OK); + az_pair header = { 0 }; + az_result fail_header_result = az_http_response_get_next_header(&response, &header); + assert_true(AZ_ERROR_HTTP_CORRUPT_RESPONSE_HEADER == fail_header_result); + } + { + az_http_response response = { 0 }; + assert_return_code( + az_http_response_init( + &response, + AZ_SPAN_FROM_STR("HTTP/1.1 404 Not Found\r\n" + "Header11: Value11\n")), + AZ_OK); + az_http_response_status_line status_line = { 0 }; assert_return_code(az_http_response_get_status_line(&response, &status_line), AZ_OK); az_pair header = { 0 }; diff --git a/sdk/tests/core/test_az_logging.c b/sdk/tests/core/test_az_logging.c index c18462102..43e849d61 100644 --- a/sdk/tests/core/test_az_logging.c +++ b/sdk/tests/core/test_az_logging.c @@ -212,10 +212,73 @@ static void test_az_log(void** state) az_log_set_callback(NULL); } +static void _log_listener_stop_logging_corrupted_response( + az_log_classification classification, + az_span message) +{ + (void)message; + switch (classification) + { + case AZ_LOG_HTTP_REQUEST: + _log_invoked_for_http_request = true; + assert_string_equal( + az_span_ptr(message), + az_span_ptr(AZ_SPAN_FROM_STR("HTTP Request : GET https://www.example.com"))); + break; + case AZ_LOG_HTTP_RESPONSE: + _log_invoked_for_http_response = true; + assert_string_equal( + az_span_ptr(message), + az_span_ptr(AZ_SPAN_FROM_STR("HTTP Response (3456ms) : 404 Not Found"))); + break; + default: + assert_true(false); + break; + } +} + +static void test_az_log_corrupted_response(void** state) +{ + (void)state; + uint8_t headers[1024] = { 0 }; + az_http_request request = { 0 }; + az_span url = AZ_SPAN_FROM_STR("https://www.example.com"); + TEST_EXPECT_SUCCESS(az_http_request_init( + &request, + &az_context_application, + az_http_method_get(), + url, + az_span_size(url), + AZ_SPAN_FROM_BUFFER(headers), + AZ_SPAN_FROM_STR("AAAAABBBBBCCCCCDDDDDEEEEEFFFFFGGGGGHHHHHIIIIIJJJJJKKKKK"))); + + az_span response_span = AZ_SPAN_FROM_STR("HTTP/1.1 404 Not Found\r\n" + "key:\n"); + az_http_response response = { 0 }; + TEST_EXPECT_SUCCESS(az_http_response_init(&response, response_span)); + + _reset_log_invocation_status(); + az_log_set_callback(_log_listener_stop_logging_corrupted_response); + assert_true(_log_invoked_for_http_request == false); + assert_true(_log_invoked_for_http_response == false); + + _az_http_policy_logging_log_http_request(&request); + assert_true(_log_invoked_for_http_request == _az_BUILT_WITH_LOGGING(true, false)); + assert_true(_log_invoked_for_http_response == false); + + _az_http_policy_logging_log_http_response(&response, 3456, &request); + assert_true(_log_invoked_for_http_request == _az_BUILT_WITH_LOGGING(true, false)); + assert_true(_log_invoked_for_http_response == _az_BUILT_WITH_LOGGING(true, false)); + + az_log_set_classifications(NULL); + az_log_set_callback(NULL); +} + int test_az_logging() { const struct CMUnitTest tests[] = { cmocka_unit_test(test_az_log), + cmocka_unit_test(test_az_log_corrupted_response), }; return cmocka_run_group_tests_name("az_core_logging", tests, NULL, NULL); }