You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2016/04/14 06:31:07 UTC

[trafficserver] branch master updated: TS-4321: Keep response headers in FetchSM as they are (#551)

This is an automated email from the ASF dual-hosted git repository.

maskit pushed a commit to branch master
in repository https://git-dual.apache.org/repos/asf/trafficserver.git

The following commit(s) were added to refs/heads/master by this push:
       new  60d07be   TS-4321: Keep response headers in FetchSM as they are (#551)
60d07be is described below

commit 60d07be8b199cc843c5e220ac0f6ed0545040422
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Thu Apr 14 13:31:00 2016 +0900

    TS-4321: Keep response headers in FetchSM as they are (#551)
---
 proxy/http2/HTTP2.cc                | 27 +++++++++++----------------
 proxy/http2/HTTP2.h                 |  2 +-
 proxy/http2/Http2ConnectionState.cc | 12 ++++++++----
 3 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index f563210..3557cec 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -499,24 +499,21 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
 }
 
 void
-http2_convert_header_from_1_1_to_2(HTTPHdr *headers)
+http2_generate_h2_header_from_1_1(HTTPHdr *headers, HTTPHdr *h2_headers)
 {
-  HTTPHdr tmp;
-  tmp.create(http_hdr_type_get(headers->m_http));
-  tmp.copy(headers);
-  headers->fields_clear();
+  h2_headers->create(http_hdr_type_get(headers->m_http));
 
-  if (http_hdr_type_get(tmp.m_http) == HTTP_TYPE_RESPONSE) {
+  if (http_hdr_type_get(headers->m_http) == HTTP_TYPE_RESPONSE) {
     char status_str[HTTP2_LEN_STATUS_VALUE_STR + 1];
-    snprintf(status_str, sizeof(status_str), "%d", tmp.status_get());
+    snprintf(status_str, sizeof(status_str), "%d", headers->status_get());
 
     // Add ':status' header field
-    MIMEField *status_field = headers->field_create(HTTP2_VALUE_STATUS, HTTP2_LEN_STATUS);
-    status_field->value_set(headers->m_heap, headers->m_mime, status_str, HTTP2_LEN_STATUS_VALUE_STR);
-    headers->field_attach(status_field);
+    MIMEField *status_field = h2_headers->field_create(HTTP2_VALUE_STATUS, HTTP2_LEN_STATUS);
+    status_field->value_set(h2_headers->m_heap, h2_headers->m_mime, status_str, HTTP2_LEN_STATUS_VALUE_STR);
+    h2_headers->field_attach(status_field);
 
     MIMEFieldIter field_iter;
-    for (MIMEField *field = tmp.iter_get_first(&field_iter); field != NULL; field = tmp.iter_get_next(&field_iter)) {
+    for (MIMEField *field = headers->iter_get_first(&field_iter); field != NULL; field = headers->iter_get_next(&field_iter)) {
       // Intermediaries SHOULD remove connection-specific header fields.
       const char *name;
       int name_len;
@@ -533,14 +530,12 @@ http2_convert_header_from_1_1_to_2(HTTPHdr *headers)
 
       MIMEField *newfield;
       name = field->name_get(&name_len);
-      newfield = headers->field_create(name, name_len);
+      newfield = h2_headers->field_create(name, name_len);
       value = field->value_get(&value_len);
-      newfield->value_set(headers->m_heap, headers->m_mime, value, value_len);
-      tmp.field_delete(field);
-      headers->field_attach(newfield);
+      newfield->value_set(h2_headers->m_heap, h2_headers->m_mime, value, value_len);
+      h2_headers->field_attach(newfield);
     }
   }
-  tmp.destroy();
 }
 
 Http2ErrorCode
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
index a70703c..7448ed6 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -331,7 +331,7 @@ Http2ErrorCode http2_decode_header_blocks(HTTPHdr *, const uint8_t *, const uint
 Http2ErrorCode http2_encode_header_blocks(HTTPHdr *, uint8_t *, uint32_t, uint32_t *, HpackHandle &);
 
 MIMEParseResult http2_convert_header_from_2_to_1_1(HTTPHdr *);
-void http2_convert_header_from_1_1_to_2(HTTPHdr *);
+void http2_generate_h2_header_from_1_1(HTTPHdr *headers, HTTPHdr *h2_headers);
 
 
 // Not sure where else to put this, but figure this is as good of a start as
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 713b91f..04b1cc4 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1007,14 +1007,17 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
 
   DebugHttp2Stream(ua_session, stream->get_id(), "Send HEADERS frame");
 
-  http2_convert_header_from_1_1_to_2(resp_header);
-  buf_len = resp_header->length_get() * 2; // Make it double just in case
+  HTTPHdr h2_hdr;
+  http2_generate_h2_header_from_1_1(resp_header, &h2_hdr);
+  buf_len = h2_hdr.length_get() * 2; // Make it double just in case
   buf = (uint8_t *)ats_malloc(buf_len);
   if (buf == NULL) {
+    h2_hdr.destroy();
     return;
   }
-  Http2ErrorCode result = http2_encode_header_blocks(resp_header, buf, buf_len, &header_blocks_size, *(this->remote_hpack_handle));
+  Http2ErrorCode result = http2_encode_header_blocks(&h2_hdr, buf, buf_len, &header_blocks_size, *(this->remote_hpack_handle));
   if (result != HTTP2_ERROR_NO_ERROR) {
+    h2_hdr.destroy();
     ats_free(buf);
     return;
   }
@@ -1023,7 +1026,7 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
   if (header_blocks_size <= BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]) - HTTP2_FRAME_HEADER_LEN) {
     payload_length = header_blocks_size;
     flags |= HTTP2_FLAGS_HEADERS_END_HEADERS;
-    if (resp_header->presence(MIME_PRESENCE_CONTENT_LENGTH) && resp_header->get_content_length() == 0) {
+    if (h2_hdr.presence(MIME_PRESENCE_CONTENT_LENGTH) && h2_hdr.get_content_length() == 0) {
       flags |= HTTP2_FLAGS_HEADERS_END_STREAM;
     }
   } else {
@@ -1057,6 +1060,7 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
     sent += payload_length;
   }
 
+  h2_hdr.destroy();
   ats_free(buf);
 }
 

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].

Re: [trafficserver] branch master updated: TS-4321: Keep response headers in FetchSM as they are (#551)

Posted by Masakazu Kitajo <ma...@apache.org>.
On Thu, Apr 14, 2016 at 1:50 PM, James Peach <jp...@apache.org> wrote:
>
>> On Apr 13, 2016, at 9:45 PM, Masakazu Kitajo <ma...@apache.org> wrote:
>>
>> I merged a pull request on GitHub with "squash and merge" button.
>> https://github.com/blog/2141-squash-your-commits
>>
>> This makes only one commits on master like we used to. It looks better
>> than making a merge commit.
>
> Is there a rebase option? If the PR is structured as a clear patch series we ought to preserve that (while still being able to squash messy PR branches).

No, it seems there is not. We would need to treat them manually in that case.


>>
>> On Thu, Apr 14, 2016 at 1:31 PM,  <ma...@apache.org> wrote:
>>> This is an automated email from the ASF dual-hosted git repository.
>>>
>>> maskit pushed a commit to branch master
>>> in repository https://git-dual.apache.org/repos/asf/trafficserver.git
>>>
>>> The following commit(s) were added to refs/heads/master by this push:
>>>       new  60d07be   TS-4321: Keep response headers in FetchSM as they are (#551)
>>> 60d07be is described below
>>>
>>> commit 60d07be8b199cc843c5e220ac0f6ed0545040422
>>> Author: Masakazu Kitajo <ma...@apache.org>
>>> AuthorDate: Thu Apr 14 13:31:00 2016 +0900
>>>
>>>    TS-4321: Keep response headers in FetchSM as they are (#551)
>>> ---
>>> proxy/http2/HTTP2.cc                | 27 +++++++++++----------------
>>> proxy/http2/HTTP2.h                 |  2 +-
>>> proxy/http2/Http2ConnectionState.cc | 12 ++++++++----
>>> 3 files changed, 20 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
>>> index f563210..3557cec 100644
>>> --- a/proxy/http2/HTTP2.cc
>>> +++ b/proxy/http2/HTTP2.cc
>>> @@ -499,24 +499,21 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
>>> }
>>>
>>> void
>>> -http2_convert_header_from_1_1_to_2(HTTPHdr *headers)
>>> +http2_generate_h2_header_from_1_1(HTTPHdr *headers, HTTPHdr *h2_headers)
>>> {
>>> -  HTTPHdr tmp;
>>> -  tmp.create(http_hdr_type_get(headers->m_http));
>>> -  tmp.copy(headers);
>>> -  headers->fields_clear();
>>> +  h2_headers->create(http_hdr_type_get(headers->m_http));
>>>
>>> -  if (http_hdr_type_get(tmp.m_http) == HTTP_TYPE_RESPONSE) {
>>> +  if (http_hdr_type_get(headers->m_http) == HTTP_TYPE_RESPONSE) {
>>>     char status_str[HTTP2_LEN_STATUS_VALUE_STR + 1];
>>> -    snprintf(status_str, sizeof(status_str), "%d", tmp.status_get());
>>> +    snprintf(status_str, sizeof(status_str), "%d", headers->status_get());
>>>
>>>     // Add ':status' header field
>>> -    MIMEField *status_field = headers->field_create(HTTP2_VALUE_STATUS, HTTP2_LEN_STATUS);
>>> -    status_field->value_set(headers->m_heap, headers->m_mime, status_str, HTTP2_LEN_STATUS_VALUE_STR);
>>> -    headers->field_attach(status_field);
>>> +    MIMEField *status_field = h2_headers->field_create(HTTP2_VALUE_STATUS, HTTP2_LEN_STATUS);
>>> +    status_field->value_set(h2_headers->m_heap, h2_headers->m_mime, status_str, HTTP2_LEN_STATUS_VALUE_STR);
>>> +    h2_headers->field_attach(status_field);
>>>
>>>     MIMEFieldIter field_iter;
>>> -    for (MIMEField *field = tmp.iter_get_first(&field_iter); field != NULL; field = tmp.iter_get_next(&field_iter)) {
>>> +    for (MIMEField *field = headers->iter_get_first(&field_iter); field != NULL; field = headers->iter_get_next(&field_iter)) {
>>>       // Intermediaries SHOULD remove connection-specific header fields.
>>>       const char *name;
>>>       int name_len;
>>> @@ -533,14 +530,12 @@ http2_convert_header_from_1_1_to_2(HTTPHdr *headers)
>>>
>>>       MIMEField *newfield;
>>>       name = field->name_get(&name_len);
>>> -      newfield = headers->field_create(name, name_len);
>>> +      newfield = h2_headers->field_create(name, name_len);
>>>       value = field->value_get(&value_len);
>>> -      newfield->value_set(headers->m_heap, headers->m_mime, value, value_len);
>>> -      tmp.field_delete(field);
>>> -      headers->field_attach(newfield);
>>> +      newfield->value_set(h2_headers->m_heap, h2_headers->m_mime, value, value_len);
>>> +      h2_headers->field_attach(newfield);
>>>     }
>>>   }
>>> -  tmp.destroy();
>>> }
>>>
>>> Http2ErrorCode
>>> diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
>>> index a70703c..7448ed6 100644
>>> --- a/proxy/http2/HTTP2.h
>>> +++ b/proxy/http2/HTTP2.h
>>> @@ -331,7 +331,7 @@ Http2ErrorCode http2_decode_header_blocks(HTTPHdr *, const uint8_t *, const uint
>>> Http2ErrorCode http2_encode_header_blocks(HTTPHdr *, uint8_t *, uint32_t, uint32_t *, HpackHandle &);
>>>
>>> MIMEParseResult http2_convert_header_from_2_to_1_1(HTTPHdr *);
>>> -void http2_convert_header_from_1_1_to_2(HTTPHdr *);
>>> +void http2_generate_h2_header_from_1_1(HTTPHdr *headers, HTTPHdr *h2_headers);
>>>
>>>
>>> // Not sure where else to put this, but figure this is as good of a start as
>>> diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
>>> index 713b91f..04b1cc4 100644
>>> --- a/proxy/http2/Http2ConnectionState.cc
>>> +++ b/proxy/http2/Http2ConnectionState.cc
>>> @@ -1007,14 +1007,17 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
>>>
>>>   DebugHttp2Stream(ua_session, stream->get_id(), "Send HEADERS frame");
>>>
>>> -  http2_convert_header_from_1_1_to_2(resp_header);
>>> -  buf_len = resp_header->length_get() * 2; // Make it double just in case
>>> +  HTTPHdr h2_hdr;
>>> +  http2_generate_h2_header_from_1_1(resp_header, &h2_hdr);
>>> +  buf_len = h2_hdr.length_get() * 2; // Make it double just in case
>>>   buf = (uint8_t *)ats_malloc(buf_len);
>>>   if (buf == NULL) {
>>> +    h2_hdr.destroy();
>>>     return;
>>>   }
>>> -  Http2ErrorCode result = http2_encode_header_blocks(resp_header, buf, buf_len, &header_blocks_size, *(this->remote_hpack_handle));
>>> +  Http2ErrorCode result = http2_encode_header_blocks(&h2_hdr, buf, buf_len, &header_blocks_size, *(this->remote_hpack_handle));
>>>   if (result != HTTP2_ERROR_NO_ERROR) {
>>> +    h2_hdr.destroy();
>>>     ats_free(buf);
>>>     return;
>>>   }
>>> @@ -1023,7 +1026,7 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
>>>   if (header_blocks_size <= BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]) - HTTP2_FRAME_HEADER_LEN) {
>>>     payload_length = header_blocks_size;
>>>     flags |= HTTP2_FLAGS_HEADERS_END_HEADERS;
>>> -    if (resp_header->presence(MIME_PRESENCE_CONTENT_LENGTH) && resp_header->get_content_length() == 0) {
>>> +    if (h2_hdr.presence(MIME_PRESENCE_CONTENT_LENGTH) && h2_hdr.get_content_length() == 0) {
>>>       flags |= HTTP2_FLAGS_HEADERS_END_STREAM;
>>>     }
>>>   } else {
>>> @@ -1057,6 +1060,7 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
>>>     sent += payload_length;
>>>   }
>>>
>>> +  h2_hdr.destroy();
>>>   ats_free(buf);
>>> }
>>>
>>>
>>> --
>>> To stop receiving notification emails like this one, please contact
>>> ['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].
>

Re: [trafficserver] branch master updated: TS-4321: Keep response headers in FetchSM as they are (#551)

Posted by James Peach <jp...@apache.org>.
> On Apr 13, 2016, at 9:45 PM, Masakazu Kitajo <ma...@apache.org> wrote:
> 
> I merged a pull request on GitHub with "squash and merge" button.
> https://github.com/blog/2141-squash-your-commits
> 
> This makes only one commits on master like we used to. It looks better
> than making a merge commit.

Is there a rebase option? If the PR is structured as a clear patch series we ought to preserve that (while still being able to squash messy PR branches).

> 
> On Thu, Apr 14, 2016 at 1:31 PM,  <ma...@apache.org> wrote:
>> This is an automated email from the ASF dual-hosted git repository.
>> 
>> maskit pushed a commit to branch master
>> in repository https://git-dual.apache.org/repos/asf/trafficserver.git
>> 
>> The following commit(s) were added to refs/heads/master by this push:
>>       new  60d07be   TS-4321: Keep response headers in FetchSM as they are (#551)
>> 60d07be is described below
>> 
>> commit 60d07be8b199cc843c5e220ac0f6ed0545040422
>> Author: Masakazu Kitajo <ma...@apache.org>
>> AuthorDate: Thu Apr 14 13:31:00 2016 +0900
>> 
>>    TS-4321: Keep response headers in FetchSM as they are (#551)
>> ---
>> proxy/http2/HTTP2.cc                | 27 +++++++++++----------------
>> proxy/http2/HTTP2.h                 |  2 +-
>> proxy/http2/Http2ConnectionState.cc | 12 ++++++++----
>> 3 files changed, 20 insertions(+), 21 deletions(-)
>> 
>> diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
>> index f563210..3557cec 100644
>> --- a/proxy/http2/HTTP2.cc
>> +++ b/proxy/http2/HTTP2.cc
>> @@ -499,24 +499,21 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
>> }
>> 
>> void
>> -http2_convert_header_from_1_1_to_2(HTTPHdr *headers)
>> +http2_generate_h2_header_from_1_1(HTTPHdr *headers, HTTPHdr *h2_headers)
>> {
>> -  HTTPHdr tmp;
>> -  tmp.create(http_hdr_type_get(headers->m_http));
>> -  tmp.copy(headers);
>> -  headers->fields_clear();
>> +  h2_headers->create(http_hdr_type_get(headers->m_http));
>> 
>> -  if (http_hdr_type_get(tmp.m_http) == HTTP_TYPE_RESPONSE) {
>> +  if (http_hdr_type_get(headers->m_http) == HTTP_TYPE_RESPONSE) {
>>     char status_str[HTTP2_LEN_STATUS_VALUE_STR + 1];
>> -    snprintf(status_str, sizeof(status_str), "%d", tmp.status_get());
>> +    snprintf(status_str, sizeof(status_str), "%d", headers->status_get());
>> 
>>     // Add ':status' header field
>> -    MIMEField *status_field = headers->field_create(HTTP2_VALUE_STATUS, HTTP2_LEN_STATUS);
>> -    status_field->value_set(headers->m_heap, headers->m_mime, status_str, HTTP2_LEN_STATUS_VALUE_STR);
>> -    headers->field_attach(status_field);
>> +    MIMEField *status_field = h2_headers->field_create(HTTP2_VALUE_STATUS, HTTP2_LEN_STATUS);
>> +    status_field->value_set(h2_headers->m_heap, h2_headers->m_mime, status_str, HTTP2_LEN_STATUS_VALUE_STR);
>> +    h2_headers->field_attach(status_field);
>> 
>>     MIMEFieldIter field_iter;
>> -    for (MIMEField *field = tmp.iter_get_first(&field_iter); field != NULL; field = tmp.iter_get_next(&field_iter)) {
>> +    for (MIMEField *field = headers->iter_get_first(&field_iter); field != NULL; field = headers->iter_get_next(&field_iter)) {
>>       // Intermediaries SHOULD remove connection-specific header fields.
>>       const char *name;
>>       int name_len;
>> @@ -533,14 +530,12 @@ http2_convert_header_from_1_1_to_2(HTTPHdr *headers)
>> 
>>       MIMEField *newfield;
>>       name = field->name_get(&name_len);
>> -      newfield = headers->field_create(name, name_len);
>> +      newfield = h2_headers->field_create(name, name_len);
>>       value = field->value_get(&value_len);
>> -      newfield->value_set(headers->m_heap, headers->m_mime, value, value_len);
>> -      tmp.field_delete(field);
>> -      headers->field_attach(newfield);
>> +      newfield->value_set(h2_headers->m_heap, h2_headers->m_mime, value, value_len);
>> +      h2_headers->field_attach(newfield);
>>     }
>>   }
>> -  tmp.destroy();
>> }
>> 
>> Http2ErrorCode
>> diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
>> index a70703c..7448ed6 100644
>> --- a/proxy/http2/HTTP2.h
>> +++ b/proxy/http2/HTTP2.h
>> @@ -331,7 +331,7 @@ Http2ErrorCode http2_decode_header_blocks(HTTPHdr *, const uint8_t *, const uint
>> Http2ErrorCode http2_encode_header_blocks(HTTPHdr *, uint8_t *, uint32_t, uint32_t *, HpackHandle &);
>> 
>> MIMEParseResult http2_convert_header_from_2_to_1_1(HTTPHdr *);
>> -void http2_convert_header_from_1_1_to_2(HTTPHdr *);
>> +void http2_generate_h2_header_from_1_1(HTTPHdr *headers, HTTPHdr *h2_headers);
>> 
>> 
>> // Not sure where else to put this, but figure this is as good of a start as
>> diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
>> index 713b91f..04b1cc4 100644
>> --- a/proxy/http2/Http2ConnectionState.cc
>> +++ b/proxy/http2/Http2ConnectionState.cc
>> @@ -1007,14 +1007,17 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
>> 
>>   DebugHttp2Stream(ua_session, stream->get_id(), "Send HEADERS frame");
>> 
>> -  http2_convert_header_from_1_1_to_2(resp_header);
>> -  buf_len = resp_header->length_get() * 2; // Make it double just in case
>> +  HTTPHdr h2_hdr;
>> +  http2_generate_h2_header_from_1_1(resp_header, &h2_hdr);
>> +  buf_len = h2_hdr.length_get() * 2; // Make it double just in case
>>   buf = (uint8_t *)ats_malloc(buf_len);
>>   if (buf == NULL) {
>> +    h2_hdr.destroy();
>>     return;
>>   }
>> -  Http2ErrorCode result = http2_encode_header_blocks(resp_header, buf, buf_len, &header_blocks_size, *(this->remote_hpack_handle));
>> +  Http2ErrorCode result = http2_encode_header_blocks(&h2_hdr, buf, buf_len, &header_blocks_size, *(this->remote_hpack_handle));
>>   if (result != HTTP2_ERROR_NO_ERROR) {
>> +    h2_hdr.destroy();
>>     ats_free(buf);
>>     return;
>>   }
>> @@ -1023,7 +1026,7 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
>>   if (header_blocks_size <= BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]) - HTTP2_FRAME_HEADER_LEN) {
>>     payload_length = header_blocks_size;
>>     flags |= HTTP2_FLAGS_HEADERS_END_HEADERS;
>> -    if (resp_header->presence(MIME_PRESENCE_CONTENT_LENGTH) && resp_header->get_content_length() == 0) {
>> +    if (h2_hdr.presence(MIME_PRESENCE_CONTENT_LENGTH) && h2_hdr.get_content_length() == 0) {
>>       flags |= HTTP2_FLAGS_HEADERS_END_STREAM;
>>     }
>>   } else {
>> @@ -1057,6 +1060,7 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
>>     sent += payload_length;
>>   }
>> 
>> +  h2_hdr.destroy();
>>   ats_free(buf);
>> }
>> 
>> 
>> --
>> To stop receiving notification emails like this one, please contact
>> ['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].


Re: [trafficserver] branch master updated: TS-4321: Keep response headers in FetchSM as they are (#551)

Posted by Masakazu Kitajo <ma...@apache.org>.
I merged a pull request on GitHub with "squash and merge" button.
https://github.com/blog/2141-squash-your-commits

This makes only one commits on master like we used to. It looks better
than making a merge commit.

On Thu, Apr 14, 2016 at 1:31 PM,  <ma...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> maskit pushed a commit to branch master
> in repository https://git-dual.apache.org/repos/asf/trafficserver.git
>
> The following commit(s) were added to refs/heads/master by this push:
>        new  60d07be   TS-4321: Keep response headers in FetchSM as they are (#551)
> 60d07be is described below
>
> commit 60d07be8b199cc843c5e220ac0f6ed0545040422
> Author: Masakazu Kitajo <ma...@apache.org>
> AuthorDate: Thu Apr 14 13:31:00 2016 +0900
>
>     TS-4321: Keep response headers in FetchSM as they are (#551)
> ---
>  proxy/http2/HTTP2.cc                | 27 +++++++++++----------------
>  proxy/http2/HTTP2.h                 |  2 +-
>  proxy/http2/Http2ConnectionState.cc | 12 ++++++++----
>  3 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
> index f563210..3557cec 100644
> --- a/proxy/http2/HTTP2.cc
> +++ b/proxy/http2/HTTP2.cc
> @@ -499,24 +499,21 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
>  }
>
>  void
> -http2_convert_header_from_1_1_to_2(HTTPHdr *headers)
> +http2_generate_h2_header_from_1_1(HTTPHdr *headers, HTTPHdr *h2_headers)
>  {
> -  HTTPHdr tmp;
> -  tmp.create(http_hdr_type_get(headers->m_http));
> -  tmp.copy(headers);
> -  headers->fields_clear();
> +  h2_headers->create(http_hdr_type_get(headers->m_http));
>
> -  if (http_hdr_type_get(tmp.m_http) == HTTP_TYPE_RESPONSE) {
> +  if (http_hdr_type_get(headers->m_http) == HTTP_TYPE_RESPONSE) {
>      char status_str[HTTP2_LEN_STATUS_VALUE_STR + 1];
> -    snprintf(status_str, sizeof(status_str), "%d", tmp.status_get());
> +    snprintf(status_str, sizeof(status_str), "%d", headers->status_get());
>
>      // Add ':status' header field
> -    MIMEField *status_field = headers->field_create(HTTP2_VALUE_STATUS, HTTP2_LEN_STATUS);
> -    status_field->value_set(headers->m_heap, headers->m_mime, status_str, HTTP2_LEN_STATUS_VALUE_STR);
> -    headers->field_attach(status_field);
> +    MIMEField *status_field = h2_headers->field_create(HTTP2_VALUE_STATUS, HTTP2_LEN_STATUS);
> +    status_field->value_set(h2_headers->m_heap, h2_headers->m_mime, status_str, HTTP2_LEN_STATUS_VALUE_STR);
> +    h2_headers->field_attach(status_field);
>
>      MIMEFieldIter field_iter;
> -    for (MIMEField *field = tmp.iter_get_first(&field_iter); field != NULL; field = tmp.iter_get_next(&field_iter)) {
> +    for (MIMEField *field = headers->iter_get_first(&field_iter); field != NULL; field = headers->iter_get_next(&field_iter)) {
>        // Intermediaries SHOULD remove connection-specific header fields.
>        const char *name;
>        int name_len;
> @@ -533,14 +530,12 @@ http2_convert_header_from_1_1_to_2(HTTPHdr *headers)
>
>        MIMEField *newfield;
>        name = field->name_get(&name_len);
> -      newfield = headers->field_create(name, name_len);
> +      newfield = h2_headers->field_create(name, name_len);
>        value = field->value_get(&value_len);
> -      newfield->value_set(headers->m_heap, headers->m_mime, value, value_len);
> -      tmp.field_delete(field);
> -      headers->field_attach(newfield);
> +      newfield->value_set(h2_headers->m_heap, h2_headers->m_mime, value, value_len);
> +      h2_headers->field_attach(newfield);
>      }
>    }
> -  tmp.destroy();
>  }
>
>  Http2ErrorCode
> diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
> index a70703c..7448ed6 100644
> --- a/proxy/http2/HTTP2.h
> +++ b/proxy/http2/HTTP2.h
> @@ -331,7 +331,7 @@ Http2ErrorCode http2_decode_header_blocks(HTTPHdr *, const uint8_t *, const uint
>  Http2ErrorCode http2_encode_header_blocks(HTTPHdr *, uint8_t *, uint32_t, uint32_t *, HpackHandle &);
>
>  MIMEParseResult http2_convert_header_from_2_to_1_1(HTTPHdr *);
> -void http2_convert_header_from_1_1_to_2(HTTPHdr *);
> +void http2_generate_h2_header_from_1_1(HTTPHdr *headers, HTTPHdr *h2_headers);
>
>
>  // Not sure where else to put this, but figure this is as good of a start as
> diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
> index 713b91f..04b1cc4 100644
> --- a/proxy/http2/Http2ConnectionState.cc
> +++ b/proxy/http2/Http2ConnectionState.cc
> @@ -1007,14 +1007,17 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
>
>    DebugHttp2Stream(ua_session, stream->get_id(), "Send HEADERS frame");
>
> -  http2_convert_header_from_1_1_to_2(resp_header);
> -  buf_len = resp_header->length_get() * 2; // Make it double just in case
> +  HTTPHdr h2_hdr;
> +  http2_generate_h2_header_from_1_1(resp_header, &h2_hdr);
> +  buf_len = h2_hdr.length_get() * 2; // Make it double just in case
>    buf = (uint8_t *)ats_malloc(buf_len);
>    if (buf == NULL) {
> +    h2_hdr.destroy();
>      return;
>    }
> -  Http2ErrorCode result = http2_encode_header_blocks(resp_header, buf, buf_len, &header_blocks_size, *(this->remote_hpack_handle));
> +  Http2ErrorCode result = http2_encode_header_blocks(&h2_hdr, buf, buf_len, &header_blocks_size, *(this->remote_hpack_handle));
>    if (result != HTTP2_ERROR_NO_ERROR) {
> +    h2_hdr.destroy();
>      ats_free(buf);
>      return;
>    }
> @@ -1023,7 +1026,7 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
>    if (header_blocks_size <= BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]) - HTTP2_FRAME_HEADER_LEN) {
>      payload_length = header_blocks_size;
>      flags |= HTTP2_FLAGS_HEADERS_END_HEADERS;
> -    if (resp_header->presence(MIME_PRESENCE_CONTENT_LENGTH) && resp_header->get_content_length() == 0) {
> +    if (h2_hdr.presence(MIME_PRESENCE_CONTENT_LENGTH) && h2_hdr.get_content_length() == 0) {
>        flags |= HTTP2_FLAGS_HEADERS_END_STREAM;
>      }
>    } else {
> @@ -1057,6 +1060,7 @@ Http2ConnectionState::send_headers_frame(FetchSM *fetch_sm)
>      sent += payload_length;
>    }
>
> +  h2_hdr.destroy();
>    ats_free(buf);
>  }
>
>
> --
> To stop receiving notification emails like this one, please contact
> ['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].