Remove accessing to _internal fields in curl transport adapter (Azure…

…#629)

* Allow empty header

* test for supporting empty headers

* adding docs

* enhance docs

* fix documentation for code

* fix docs for function

* remove internal access from transport to response

* adding docs for functions

* rename function to be public

* Apply suggestions from code review

Co-Authored-By: Ahson Khan <ahson_ahmedk@yahoo.com>

* create getters for http request parts

* Apply suggestions from code review

Co-authored-by: Ahson Khan <ahson_ahmedk@yahoo.com>

* do not add 0 at the end of responses

* fix precondition name changed

* Apply suggestions from code review

Co-authored-by: Ahson Khan <ahson_ahmedk@yahoo.com>

Co-authored-by: Ahson Khan <ahson_ahmedk@yahoo.com>

sdk/core/core/inc/az_http_transport.h

@@ -85,27 +85,88 @@ AZ_NODISCARD az_result
 az_http_request_get_header(_az_http_request const* request, int32_t index, az_pair* out_header);
 
 /**
- * @brief Get parts of an HTTP request. `NULL` in accepted to ignore getting any parts, for example,
- * call this function like below to get only the http method and ignore getting url and body.
- * `az_http_request_get_parts(request, &method, NULL, NULL)`
+ * @brief Get method of an HTTP request.
  *
- * This function is expected to be used by transport layer only.
+ * @remarks This function is expected to be used by transport layer only.
  *
- * @param request HTTP request to get parts from.
- * @param[out] out_method __[nullable]__ Pointer to write HTTP method to. Use `NULL` to ignore
- * getting this value.
- * @param[out] out_url __[nullable]__ Pointer to write URL to. Use `NULL` to ignore getting this
- * value.
- * @param[out] out_body __[nullable]__ Pointer to write HTTP request body to. Use `NULL` to ignore
- * getting this value.
+ * @param[in] request The HTTP request from which to get the method.
+ * @param[out] out_method Pointer to write the HTTP method to.
  *
- * @retval AZ_OK Success.
+ * @retval An #az_result value indicating the result of the operation:
+ * - #AZ_OK if successful
+ */
+AZ_NODISCARD az_result
+az_http_request_get_method(_az_http_request const* request, az_http_method* out_method);
+
+/**
+ * @brief Get url from an HTTP request.
+ *
+ * @remarks This function is expected to be used by transport layer only.
+ *
+ * @param[in] request The HTTP request from which to get the url.
+ * @param[out] out_url Pointer to write the HTTP URL to.
+ *
+ * @retval An #az_result value indicating the result of the operation:
+ * - #AZ_OK if successful
  */
-AZ_NODISCARD az_result az_http_request_get_parts(
- _az_http_request const* request,
- az_http_method* out_method,
- az_span* out_url,
- az_span* out_body);
+AZ_NODISCARD az_result az_http_request_get_url(_az_http_request const* request, az_span* out_url);
+
+/**
+ * @brief Get body from an HTTP request.
+ *
+ * @remarks This function is expected to be used by transport layer only.
+ *
+ * @param[in] request The HTTP request from which to get the body.
+ * @param[out] out_body Pointer to write the HTTP request body to.
+ *
+ * @retval An #az_result value indicating the result of the operation:
+ * - #AZ_OK if successful
+ */
+AZ_NODISCARD az_result az_http_request_get_body(_az_http_request const* request, az_span* out_body);
+
+/**
+ * @brief This function is expected to be used by transport adapters like curl. Use it to write
+ * content from \p source to \p response.
+ *
+ * @remarks The \p source can be an empty #az_span. If so, nothing will be written.
+ *
+ * @param[in] response Pointer to an az_http_response.
+ * @param[in] source This is an az_span with the content to be written into response.
+ * @return An #az_result value indicating the result of the operation:
+ * - #AZ_OK if successful
+ * - #AZ_ERROR_INSUFFICIENT_SPAN_SIZE if the \p response buffer is not big enough to contain
+ * the \p source content
+ */
+AZ_NODISCARD az_result az_http_response_write_span(az_http_response* response, az_span source);
+
+/**
+ * @brief Returns the number of headers within the request.
+ * Each header is an #az_pair.
+ *
+ * @param[in] request Pointer to an az_http_request to be used by this function.
+ * @return Number of headers in the request.
+ */
+AZ_NODISCARD int32_t az_http_request_headers_count(_az_http_request const* request);
+
+/**
+ * @brief Send an HTTP request through the wire and write the response into \p p_response.
+ *
+ * @param[in] p_request Points to an az_http_request that contains the settings and data that is
+ * used to send the request through the wire.
+ * @param[out] p_response Points to an az_http_response where the response from the wire will be
+ * written.
+ * @return An #az_result value indicating the result of the operation:
+ * - #AZ_OK if successful
+ * - #AZ_ERROR_HTTP_RESPONSE_OVERFLOW if there was any issue while trying to write into \p p_response. It
+ * might mean that there was not enough space in \p p_response to hold the entire response from
+ * the network.
+ * - #AZ_ERROR_HTTP_RESPONSE_COULDNT_RESOLVE_HOST if the url from \p p_request can't be resolved by the http
+ * stack and the request was not sent.
+ * - #AZ_ERROR_HTTP_PLATFORM any other issue from the transport layer.
+ *
+ */
+AZ_NODISCARD az_result
+az_http_client_send_request(_az_http_request* p_request, az_http_response* p_response);
 
 #include <_az_cfg_suffix.h>
 

sdk/core/core/internal/az_http_internal.h

@@ -112,16 +112,6 @@ _az_http_policy_apiversion_options_default()
  return (_az_http_policy_apiversion_options){ 0 };
 }
 
-/**
- * @brief Returns the count of headers on the request
- * Each header is an az_pair
- *
- */
-AZ_NODISCARD AZ_INLINE int32_t _az_http_request_headers_count(_az_http_request const* request)
-{
- return request->_internal.headers_length;
-}
-
 /**
  * @brief Initialize az_http_policy_retry_options with default values
  *
@@ -186,9 +176,6 @@ AZ_NODISCARD az_result az_http_pipeline_policy_transport(
  _az_http_request* p_request,
  az_http_response* p_response);
 
-AZ_NODISCARD az_result
-az_http_client_send_request(_az_http_request* p_request, az_http_response* p_response);
-
 /**
  * @brief Format buffer as a http request containing URL and header spans.
  *

sdk/core/core/src/az_http_policy_logging.c

@@ -87,7 +87,7 @@ static az_result _az_http_policy_logging_append_http_request_msg(
  remainder = az_span_copy(
  remainder, az_span_slice(request->_internal.url, 0, request->_internal.url_length));
 
- int32_t const headers_count = _az_http_request_headers_count(request);
+ int32_t const headers_count = az_http_request_headers_count(request);
 
  az_span new_line_tab_string = AZ_SPAN_FROM_STR("\n\t");
  az_span colon_separator_string = AZ_SPAN_FROM_STR(" : ");

sdk/core/core/src/az_http_request.c

@@ -170,7 +170,7 @@ az_http_request_get_header(_az_http_request const* request, int32_t index, az_pa
  _az_PRECONDITION_NOT_NULL(request);
  _az_PRECONDITION_NOT_NULL(out_header);
 
- if (index >= _az_http_request_headers_count(request))
+ if (index >= az_http_request_headers_count(request))
  {
  return AZ_ERROR_ARG;
  }
@@ -179,23 +179,37 @@ az_http_request_get_header(_az_http_request const* request, int32_t index, az_pa
  return AZ_OK;
 }
 
-AZ_NODISCARD az_result az_http_request_get_parts(
- _az_http_request const* request,
- az_http_method* out_method,
- az_span* out_url,
- az_span* out_body)
+AZ_NODISCARD az_result
+az_http_request_get_method(_az_http_request const* request, az_http_method* out_method)
 {
- if (out_method != NULL)
- {
- *out_method = request->_internal.method;
- }
- if (out_url != NULL)
- {
- *out_url = az_span_slice(request->_internal.url, 0, request->_internal.url_length);
- }
- if (out_body != NULL)
- {
- *out_body = request->_internal.body;
- }
+ _az_PRECONDITION_NOT_NULL(request);
+ _az_PRECONDITION_NOT_NULL(out_method);
+
+ *out_method = request->_internal.method;
+
+ return AZ_OK;
+}
+
+AZ_NODISCARD az_result az_http_request_get_url(_az_http_request const* request, az_span* out_url)
+{
+ _az_PRECONDITION_NOT_NULL(request);
+ _az_PRECONDITION_NOT_NULL(out_url);
+
+ *out_url = az_span_slice(request->_internal.url, 0, request->_internal.url_length);
+
  return AZ_OK;
 }
+
+AZ_NODISCARD az_result az_http_request_get_body(_az_http_request const* request, az_span* out_body)
+{
+ _az_PRECONDITION_NOT_NULL(request);
+ _az_PRECONDITION_NOT_NULL(out_body);
+
+ *out_body = request->_internal.body;
+ return AZ_OK;
+}
+
+AZ_NODISCARD int32_t az_http_request_headers_count(_az_http_request const* request)
+{
+ return request->_internal.headers_length;
+}

sdk/core/core/src/az_http_response.c

@@ -268,3 +268,23 @@ void _az_http_response_reset(az_http_response* http_response)
  az_result result = az_http_response_init(http_response, http_response->_internal.http_response);
  (void)result;
 }
+
+// internal function to get az_http_response remainder
+static az_span _az_http_response_get_remaining(az_http_response const* response)
+{
+ return az_span_slice_to_end(response->_internal.http_response, response->_internal.written);
+}
+
+AZ_NODISCARD az_result az_http_response_write_span(az_http_response* response, az_span source)
+{
+ _az_PRECONDITION_NOT_NULL(response);
+
+ az_span remaining = _az_http_response_get_remaining(response);
+ int32_t write_size = az_span_size(source);
+ AZ_RETURN_IF_NOT_ENOUGH_SIZE(remaining, write_size);
+
+ remaining = az_span_copy(remaining, source);
+ response->_internal.written += write_size;
+
+ return AZ_OK;
+}

sdk/core/core/test/cmocka/test_az_http.c

@@ -139,7 +139,9 @@ static void test_http_request(void** state)
  az_http_method method;
  az_span body;
  az_span url;
- assert_return_code(az_http_request_get_parts(&hrb, &method, &url, &body), AZ_OK);
+ assert_return_code(az_http_request_get_method(&hrb, &method), AZ_OK);
+ assert_return_code(az_http_request_get_url(&hrb, &url), AZ_OK);
+ assert_return_code(az_http_request_get_body(&hrb, &body), AZ_OK);
  assert_string_equal(az_span_ptr(method), az_span_ptr(az_http_method_get()));
  assert_string_equal(az_span_ptr(body), az_span_ptr(AZ_SPAN_FROM_STR("body")));
  assert_string_equal(

sdk/platform/http_client/curl/src/az_curl.c

@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: MIT
 
 #include <az_http.h>
-#include <az_http_internal.h>
 #include <az_http_transport.h>
 #include <az_span.h>
 
@@ -224,7 +223,7 @@ _az_http_client_curl_build_headers(_az_http_request* p_request, struct curl_slis
  _az_PRECONDITION_NOT_NULL(p_request);
 
  az_pair header;
- for (int32_t offset = 0; offset < _az_http_request_headers_count(p_request); ++offset)
+ for (int32_t offset = 0; offset < az_http_request_headers_count(p_request); ++offset)
  {
  AZ_RETURN_IF_FAILED(az_http_request_get_header(p_request, offset, &header));
  AZ_RETURN_IF_FAILED(
@@ -275,19 +274,16 @@ static size_t _az_http_client_curl_write_to_span(
  size_t const expected_size = size * nmemb;
  az_http_response* response = (az_http_response*)userp;
 
- az_span remaining
- = az_span_slice_to_end(response->_internal.http_response, response->_internal.written);
-
  az_span const span_for_content = az_span_init((uint8_t*)contents, (int32_t)expected_size);
 
- if (az_span_size(remaining) < (int32_t)expected_size)
+ az_result write_response_result = az_http_response_write_span(response, span_for_content);
+
+ if (az_failed(write_response_result))
  {
- return expected_size + 1;
+ return expected_size
+ + 1; // Adding any constant to return value will tell curl that this function failed
  }
 
- az_span_copy(remaining, span_for_content);
- response->_internal.written += (int32_t)expected_size;
-
  // This callback needs to return the response size or curl will consider it as it failed
  return expected_size;
 }
@@ -332,14 +328,15 @@ _az_http_client_curl_send_post_request(CURL* p_curl, _az_http_request const* p_r
  _az_PRECONDITION_NOT_NULL(p_request);
 
  // Method
+ az_span request_body = { 0 };
+ AZ_RETURN_IF_FAILED(az_http_request_get_body(p_request, &request_body));
  az_span body = { 0 };
- int32_t const required_length
- = az_span_size(p_request->_internal.body) + az_span_size(AZ_SPAN_FROM_STR("\0"));
+ int32_t const required_length = az_span_size(request_body) + az_span_size(AZ_SPAN_FROM_STR("\0"));
 
  AZ_RETURN_IF_FAILED(_az_span_malloc(required_length, &body));
 
  char* b = (char*)az_span_ptr(body);
- az_span_to_str(b, required_length, p_request->_internal.body);
+ az_span_to_str(b, required_length, request_body);
 
  az_result res_code
  = _az_http_client_curl_code_to_result(curl_easy_setopt(p_curl, CURLOPT_POSTFIELDS, b));
@@ -415,7 +412,8 @@ _az_http_client_curl_send_upload_request(CURL* p_curl, _az_http_request const* p
  _az_PRECONDITION_NOT_NULL(p_curl);
  _az_PRECONDITION_NOT_NULL(p_request);
 
- az_span body = p_request->_internal.body;
+ az_span body = { 0 };
+ AZ_RETURN_IF_FAILED(az_http_request_get_body(p_request, &body));
 
  AZ_RETURN_IF_CURL_FAILED(curl_easy_setopt(p_curl, CURLOPT_UPLOAD, 1L));
  AZ_RETURN_IF_CURL_FAILED(
@@ -451,7 +449,7 @@ static AZ_NODISCARD az_result _az_http_client_curl_setup_headers(
  _az_PRECONDITION_NOT_NULL(p_curl);
  _az_PRECONDITION_NOT_NULL(p_request);
 
- if (_az_http_request_headers_count(p_request) == 0)
+ if (az_http_request_headers_count(p_request) == 0)
  {
  // no headers, no need to set it up
  return AZ_OK;
@@ -478,18 +476,23 @@ _az_http_client_curl_setup_url(CURL* p_curl, _az_http_request const* p_request)
  _az_PRECONDITION_NOT_NULL(p_curl);
  _az_PRECONDITION_NOT_NULL(p_request);
 
+ az_span request_url = { 0 };
+ // get request_url. It will have the size of what it has written in it only
+ AZ_RETURN_IF_FAILED(az_http_request_get_url(p_request, &request_url));
+ int32_t request_url_size = az_span_size(request_url);
+
  az_span writable_buffer;
  {
  // Add 1 for 0-terminated str
- int32_t const url_final_size = p_request->_internal.url_length + 1;
+ int32_t const url_final_size = request_url_size + 1;
 
  // allocate buffer to add \0
  AZ_RETURN_IF_FAILED(_az_span_malloc(url_final_size, &writable_buffer));
  }
 
  // write url in buffer (will add \0 at the end)
- az_result result = _az_http_client_curl_append_url(
-  writable_buffer, az_span_slice(p_request->_internal.url, 0, p_request->_internal.url_length));
+ // request_url is already the right size containing only what has been written into it
+ az_result result = _az_http_client_curl_append_url(writable_buffer, request_url);
 
  if (az_succeeded(result))
  {
@@ -557,20 +560,23 @@ static AZ_NODISCARD az_result _az_http_client_curl_send_request_impl_process(
 
  AZ_RETURN_IF_FAILED(_az_http_client_curl_setup_response_redirect(p_curl, response));
 
- if (az_span_is_content_equal(p_request->_internal.method, az_http_method_get()))
+ az_http_method method;
+ AZ_RETURN_IF_FAILED(az_http_request_get_method(p_request, &method));
+
+ if (az_span_is_content_equal(method, az_http_method_get()))
  {
  result = _az_http_client_curl_send_get_request(p_curl);
  }
- else if (az_span_is_content_equal(p_request->_internal.method, az_http_method_delete()))
+ else if (az_span_is_content_equal(method, az_http_method_delete()))
  {
  result = _az_http_client_curl_send_delete_request(p_curl, p_request);
  }
- else if (az_span_is_content_equal(p_request->_internal.method, az_http_method_post()))
+ else if (az_span_is_content_equal(method, az_http_method_post()))
  {
  AZ_RETURN_IF_FAILED(_az_http_client_curl_add_expect_header(p_curl, &p_list));
  result = _az_http_client_curl_send_post_request(p_curl, p_request);
  }
- else if (az_span_is_content_equal(p_request->_internal.method, az_http_method_put()))
+ else if (az_span_is_content_equal(method, az_http_method_put()))
  {
  // As of CURL 7.12.1 CURLOPT_PUT is deprecated. PUT requests should be made using
  // CURLOPT_UPLOAD
@@ -585,17 +591,6 @@ static AZ_NODISCARD az_result _az_http_client_curl_send_request_impl_process(
  // Clean custom headers previously appended
  curl_slist_free_all(p_list);
 
- // make sure to set the end of the body response as the end of the complete response
- if (az_succeeded(result))
- {
- az_span remaining
- = az_span_slice_to_end(response->_internal.http_response, response->_internal.written);
-
- AZ_RETURN_IF_NOT_ENOUGH_SIZE(remaining, 1);
-
- az_span_copy_u8(remaining, 0);
- response->_internal.written++;
- }
  return result;
 }