[
https://issues.apache.org/jira/browse/TS-3845?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Bryan Call updated TS-3845:
---------------------------
Fix Version/s: (was: 6.1.0)
6.0.0
> HPACK error when trying to delete entries from an empty table
> -------------------------------------------------------------
>
> Key: TS-3845
> URL: https://issues.apache.org/jira/browse/TS-3845
> Project: Traffic Server
> Issue Type: Improvement
> Components: HTTP/2
> Reporter: Bryan Call
> Assignee: Leif Hedstrom
> Fix For: 6.0.0
>
>
> This coredump that happens very seldom. It happens when
> Http2DynamicTable::add_header_field() is called and it tires to delete
> entries from an empty table to make room.
> Here is a patch to stop the core from happening, but it would be better to
> find the root cause. I am running another patch in production that walks the
> table on every add and calculates the used space and compares it to
> _current_size to see if I can diagnose it more.
> {code}
> diff --git a/proxy/http2/HPACK.cc b/proxy/http2/HPACK.cc
> index d65a6b1..76e3877 100644
> --- a/proxy/http2/HPACK.cc
> +++ b/proxy/http2/HPACK.cc
> @@ -230,7 +230,7 @@ Http2DynamicTable::set_dynamic_table_size(uint32_t
> new_size)
> return true;
> }
> -void
> +bool
> Http2DynamicTable::add_header_field(const MIMEField *field)
> {
> int name_len, value_len;
> @@ -247,7 +247,7 @@ Http2DynamicTable::add_header_field(const MIMEField
> *field)
> _mhdr->fields_clear();
> } else {
> _current_size += header_size;
> - while (_current_size > _settings_dynamic_table_size) {
> + while (_current_size > _settings_dynamic_table_size && _headers.length()
> > 0) {
> int last_name_len, last_value_len;
> MIMEField *last_field = _headers.last();
> @@ -259,11 +259,18 @@ Http2DynamicTable::add_header_field(const MIMEField
> *field)
> _mhdr->field_delete(last_field, false);
> }
> + if (_headers.length() == 0 && _current_size - header_size != 0) {
> + Error("No headers in dynamic table, but the size is: %u",
> _current_size);
> + return false; // this will close the connection
> + }
> +
> MIMEField *new_field = _mhdr->field_create(name, name_len);
> new_field->value_set(_mhdr->m_heap, _mhdr->m_mime, value, value_len);
> // XXX Because entire Vec instance is copied, Its too expensive!
> _headers.insert(0, new_field);
> }
> +
> + return true;
> }
> // The first byte of an HPACK field unambiguously tells us what
> @@ -658,7 +665,9 @@ decode_literal_header_field(MIMEFieldWrapper &header,
> const uint8_t *buf_start,
> // Incremental Indexing adds header to header table as new entry
> if (isIncremental) {
> - dynamic_table.add_header_field(header.field_get());
> + if (dynamic_table.add_header_field(header.field_get()) == false) {
> + return HPACK_ERROR_COMPRESSION_ERROR;
> + }
> }
> // Print decoded header field
> diff --git a/proxy/http2/HPACK.h b/proxy/http2/HPACK.h
> index 4e63a37..e03ef1a 100644
> --- a/proxy/http2/HPACK.h
> +++ b/proxy/http2/HPACK.h
> @@ -111,7 +111,7 @@ public:
> delete _mhdr;
> }
> - void add_header_field(const MIMEField *field);
> + bool add_header_field(const MIMEField *field);
> int get_header_from_indexing_tables(uint32_t index, MIMEFieldWrapper
> &header_field) const;
> bool set_dynamic_table_size(uint32_t new_size);
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
