Switch to generic xml unescaping logic (#631)

Author: DmitriyMusatkin

.github/workflows/ci.yml

@@ -42,7 +42,7 @@ jobs:
         role-to-assume: ${{ env.CRT_CI_ROLE }}
         aws-region: ${{ env.AWS_DEFAULT_REGION }}
     - name: Checkout Sources
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
         # We can't use the `uses: docker://image` version yet, GitHub lacks authentication for actions -> packages
     - name: Build ${{ env.PACKAGE_NAME }}
       run: |
@@ -204,7 +204,7 @@ jobs:
         role-to-assume: ${{ env.CRT_CI_ROLE }}
         aws-region: ${{ env.AWS_DEFAULT_REGION }}
     - name: Checkout Sources
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
     - name: Build ${{ env.PACKAGE_NAME }} + consumers
       run: |
         python3 -m venv .venv
@@ -221,7 +221,7 @@ jobs:
         role-to-assume: ${{ env.CRT_CI_ROLE }}
         aws-region: ${{ env.AWS_DEFAULT_REGION }}
     - name: Checkout Sources
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
     - name: Build ${{ env.PACKAGE_NAME }} + consumers
       run: |
         python3 -m venv .venv

.github/workflows/clang-format.yml

@@ -9,7 +9,7 @@ jobs:
 
     steps:
     - name: Checkout Sources
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
 
     - name: clang-format lint
       run: |

.github/workflows/codecov.yml

@@ -25,7 +25,7 @@ jobs:
         role-to-assume: ${{ env.CRT_CI_ROLE }}
         aws-region: ${{ env.AWS_DEFAULT_REGION }}
     - name: Checkout Sources
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
     - name: Build ${{ env.PACKAGE_NAME }} + consumers
       run: |
         python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"

include/aws/s3/private/s3_util.h

@@ -236,11 +236,6 @@ int aws_xml_get_body_at_path(
     const char *path_name_array[],
     struct aws_byte_cursor *out_body);
 
-/* replace " with escaped /"
- * Returns initialized aws_byte_buf */
-AWS_S3_API
-struct aws_byte_buf aws_replace_quote_entities(struct aws_allocator *allocator, struct aws_byte_cursor src);
-
 /* strip quotes if string is enclosed in quotes. does not remove quotes if they only appear on either side of the string
  */
 AWS_S3_API

source/s3_auto_ranged_put.c

@@ -13,6 +13,7 @@
 #include <aws/common/clock.h>
 #include <aws/common/encoding.h>
 #include <aws/common/string.h>
+#include <aws/common/xml_parser.h>
 #include <aws/io/stream.h>
 
 /* TODO: better logging of steps */
@@ -1756,13 +1757,20 @@ static void s_s3_auto_ranged_put_request_finished(
                 aws_xml_get_body_at_path(meta_request->allocator, xml_doc, xml_path, &etag_header_value);
 
                 if (etag_header_value.len > 0) {
-                    struct aws_byte_buf etag_header_value_byte_buf =
-                        aws_replace_quote_entities(meta_request->allocator, etag_header_value);
-
-                    aws_http_headers_set(
-                        request->send_data.response_headers,
-                        g_etag_header_name,
-                        aws_byte_cursor_from_buf(&etag_header_value_byte_buf));
+                    struct aws_byte_buf etag_header_value_byte_buf;
+                    aws_byte_buf_init(&etag_header_value_byte_buf, meta_request->allocator, 20);
+                    if (aws_byte_buf_append_unescaped_xml(
+                            meta_request->allocator, etag_header_value, &etag_header_value_byte_buf)) {
+                        AWS_LOGF_ERROR(
+                            AWS_LS_S3_META_REQUEST, "Server returned unexpected etag format. skipping etag unescaping");
+                        aws_http_headers_set(
+                            request->send_data.response_headers, g_etag_header_name, etag_header_value);
+                    } else {
+                        aws_http_headers_set(
+                            request->send_data.response_headers,
+                            g_etag_header_name,
+                            aws_byte_cursor_from_buf(&etag_header_value_byte_buf));
+                    }
 
                     aws_byte_buf_clean_up(&etag_header_value_byte_buf);
                 }

source/s3_copy_object.c

@@ -8,6 +8,7 @@
 #include "aws/s3/private/s3_request_messages.h"
 #include "aws/s3/private/s3_util.h"
 #include <aws/common/string.h>
+#include <aws/common/xml_parser.h>
 
 /* Objects with size smaller than the constant below are bypassed as S3 CopyObject instead of multipart copy */
 static const size_t s_multipart_copy_minimum_object_size = GB_TO_BYTES(1);
@@ -569,7 +570,9 @@ static struct aws_string *s_etag_new_from_upload_part_copy_response(
     const char *xml_path[] = {"CopyPartResult", "ETag", NULL};
     aws_xml_get_body_at_path(allocator, xml_doc, xml_path, &etag_within_xml_quotes);
 
-    struct aws_byte_buf etag_within_quotes_byte_buf = aws_replace_quote_entities(allocator, etag_within_xml_quotes);
+    struct aws_byte_buf etag_within_quotes_byte_buf;
+    aws_byte_buf_init(&etag_within_quotes_byte_buf, allocator, 20);
+    aws_byte_buf_append_unescaped_xml(allocator, etag_within_xml_quotes, &etag_within_quotes_byte_buf);
 
     struct aws_string *stripped_etag =
         aws_strip_quotes(allocator, aws_byte_cursor_from_buf(&etag_within_quotes_byte_buf));
@@ -770,13 +773,20 @@ static void s_s3_copy_object_request_finished(
                 const char *xml_path[] = {"CompleteMultipartUploadResult", "ETag", NULL};
                 aws_xml_get_body_at_path(meta_request->allocator, xml_doc, xml_path, &etag_header_value);
                 if (etag_header_value.len > 0) {
-                    struct aws_byte_buf etag_header_value_byte_buf =
-                        aws_replace_quote_entities(meta_request->allocator, etag_header_value);
-
-                    aws_http_headers_set(
-                        request->send_data.response_headers,
-                        g_etag_header_name,
-                        aws_byte_cursor_from_buf(&etag_header_value_byte_buf));
+                    struct aws_byte_buf etag_header_value_byte_buf;
+                    aws_byte_buf_init(&etag_header_value_byte_buf, meta_request->allocator, 20);
+                    if (aws_byte_buf_append_unescaped_xml(
+                            meta_request->allocator, etag_header_value, &etag_header_value_byte_buf)) {
+                        AWS_LOGF_ERROR(
+                            AWS_LS_S3_META_REQUEST, "Server returned unexpected etag format. skipping etag unescaping");
+                        aws_http_headers_set(
+                            request->send_data.response_headers, g_etag_header_name, etag_header_value);
+                    } else {
+                        aws_http_headers_set(
+                            request->send_data.response_headers,
+                            g_etag_header_name,
+                            aws_byte_cursor_from_buf(&etag_header_value_byte_buf));
+                    }
 
                     aws_byte_buf_clean_up(&etag_header_value_byte_buf);
                 }

source/s3_list_objects.c

@@ -131,8 +131,13 @@ static int s_on_list_bucket_result_node_encountered(struct aws_xml_node *node, v
             fs_wrapper.fs_info.prefix = aws_byte_cursor_from_string(operation_data->prefix);
         }
 
-        struct aws_byte_buf trimmed_etag = aws_replace_quote_entities(fs_wrapper.allocator, fs_wrapper.fs_info.e_tag);
-        fs_wrapper.fs_info.e_tag = aws_byte_cursor_from_buf(&trimmed_etag);
+        struct aws_byte_buf trimmed_etag;
+        aws_byte_buf_init(&trimmed_etag, fs_wrapper.allocator, 20);
+
+        if (aws_byte_buf_append_unescaped_xml(fs_wrapper.allocator, fs_wrapper.fs_info.e_tag, &trimmed_etag) ==
+            AWS_OP_SUCCESS) {
+            fs_wrapper.fs_info.e_tag = aws_byte_cursor_from_buf(&trimmed_etag);
+        }
 
         int ret_val = AWS_OP_SUCCESS;
         if (operation_data->on_object) {

source/s3_list_parts.c

@@ -142,9 +142,13 @@ static int s_xml_on_ListPartsResult_child(struct aws_xml_node *node, void *user_
             return AWS_OP_ERR;
         }
 
-        struct aws_byte_buf trimmed_etag =
-            aws_replace_quote_entities(result_wrapper.allocator, result_wrapper.part_info.e_tag);
-        result_wrapper.part_info.e_tag = aws_byte_cursor_from_buf(&trimmed_etag);
+        struct aws_byte_buf trimmed_etag;
+        aws_byte_buf_init(&trimmed_etag, result_wrapper.allocator, 20);
+
+        if (aws_byte_buf_append_unescaped_xml(
+                result_wrapper.allocator, result_wrapper.part_info.e_tag, &trimmed_etag) == AWS_OP_SUCCESS) {
+            result_wrapper.part_info.e_tag = aws_byte_cursor_from_buf(&trimmed_etag);
+        }
 
         int ret_val = AWS_OP_SUCCESS;
         if (operation_data->on_part) {

source/s3_util.c

@@ -333,31 +333,6 @@ void aws_s3_init_default_signing_config(
     signing_config->signed_body_value = g_aws_signed_body_value_unsigned_payload;
 }
 
-static struct aws_byte_cursor s_quote_entity_literal = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL(""");
-static struct aws_byte_cursor s_quote_literal = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("\"");
-
-struct aws_byte_buf aws_replace_quote_entities(struct aws_allocator *allocator, struct aws_byte_cursor src) {
-    struct aws_byte_buf out_buf;
-    aws_byte_buf_init(&out_buf, allocator, src.len);
-
-    for (size_t i = 0; i < src.len; ++i) {
-        size_t chars_remaining = src.len - i;
-
-        if (chars_remaining >= s_quote_entity_literal.len &&
-            !strncmp((const char *)&src.ptr[i], (const char *)s_quote_entity_literal.ptr, s_quote_entity_literal.len)) {
-            /* Append quote */
-            aws_byte_buf_append(&out_buf, &s_quote_literal);
-            i += s_quote_entity_literal.len - 1;
-        } else {
-            /* Append character */
-            struct aws_byte_cursor character_cursor = aws_byte_cursor_from_array(&src.ptr[i], 1);
-            aws_byte_buf_append(&out_buf, &character_cursor);
-        }
-    }
-
-    return out_buf;
-}
-
 struct aws_string *aws_strip_quotes(struct aws_allocator *allocator, struct aws_byte_cursor in_cur) {
 
     if (in_cur.len >= 2 && in_cur.ptr[0] == '"' && in_cur.ptr[in_cur.len - 1] == '"') {

tests/s3_util_tests.c

@@ -133,7 +133,9 @@ static int s_test_s3_replace_quote_entities(struct aws_allocator *allocator, voi
     for (size_t i = 0; i < AWS_ARRAY_SIZE(test_cases); ++i) {
         struct test_case *test_case = &test_cases[i];
 
-        struct aws_byte_buf result_byte_buf = aws_replace_quote_entities(allocator, test_case->test_string);
+        struct aws_byte_buf result_byte_buf;
+        aws_byte_buf_init(&result_byte_buf, allocator, 20);
+        ASSERT_SUCCESS(aws_byte_buf_append_unescaped_xml(allocator, test_case->test_string, &result_byte_buf));
 
         struct aws_byte_cursor result_byte_cursor = aws_byte_cursor_from_buf(&result_byte_buf);