[GitHub] [trafficserver] masaori335 commented on a diff in pull request #9239: Fix ordering Via response header values

[GitBox <gi...@apache.org>]

masaori335 commented on code in PR #9239:
URL: https://github.com/apache/trafficserver/pull/9239#discussion_r1052718143


##########
proxy/http/HttpTransact.cc:
##########
@@ -4614,11 +4614,17 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s)
     //  the order of the fields
     MIMEField *resp_via = s->hdr_info.server_response.field_find(MIME_FIELD_VIA, MIME_LEN_VIA);
     if (resp_via) {
+      int saved_our_via_len = 0;
+      char saved_our_via[1024]; // 512-bytes for hostname+via string, 512-bytes for the debug info
       MIMEField *our_via;
       our_via = s->hdr_info.client_response.field_find(MIME_FIELD_VIA, MIME_LEN_VIA);
       if (our_via == nullptr) {
         our_via = s->hdr_info.client_response.field_create(MIME_FIELD_VIA, MIME_LEN_VIA);
         s->hdr_info.client_response.field_attach(our_via);
+      } else {
+        const char *src = our_via->value_get(&saved_our_via_len);
+        memcpy(saved_our_via, src, saved_our_via_len);

Review Comment:
   No heap allocation is the best choice. In this case, `1024` looks good to me.
   
   If the buffer size is dynamic, please consider using `ts::LocalBuffer` instead of `std::string`. It switches stack and heap depends on the size.
   
   https://github.com/apache/trafficserver/blob/master/include/tscpp/util/LocalBuffer.h



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org