Add AZ_ERROR_HTTP_NO_MORE_HEADERS and update az_http_response_get_next_header to return it. (#1157)

* Add AZ_ERROR_HTTP_NO_MORE_HEADERS and update
az_http_response_get_next_header to return it.

* Update logging test.

* Update changelog.

* Fix test string being compared against.

* Address PR feedback.

* Update docs and changelog.

* Add back the HTTP in fix to the name.

* Move HTTP_ADAPTER to 9 since we added a new error.

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

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.
  */

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.

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;
     }

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);

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,
     }

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 };

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);
 }