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

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

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


##########
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:
   Thanks for your review.
   
   I suppose we are safe here since the value of `our_via` is created in `HttpTransactHeaders::insert_via_header_in_response` and the size of `saved_our_via` is matched to the size of the local variable `new_via_string`.
   
   https://github.com/apache/trafficserver/blob/e8ccdb21b95a3f370be5ed974cb86ea64c435662/proxy/http/HttpTransactHeaders.cc#L842-L845
   
   I thought it would be nice to avoid a heap allocation.
   Is it better to use `std::string` here even it causes a heap allocation?
   Or keep it as is and add a comment that the size of `our_via` is matchedd to `new_via_string` in `HttpTransactHeaders::insert_via_header_in_response`?



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