trafficserver icon indicating copy to clipboard operation
trafficserver copied to clipboard

Is this missed to forward argument in `Http2CommonSession::add_url_to_pushed_table`?

Open freak82 opened this issue 1 year ago • 1 comments

Hi there,

The url_len argument of Http2CommonSession::add_url_to_pushed_table function is not passed to the emplace call.

void
Http2CommonSession::add_url_to_pushed_table(const char *url, int url_len)
{
  // Delay std::unordered_set allocation until when it used
  if (_h2_pushed_urls == nullptr) {
    this->_h2_pushed_urls = new std::unordered_set<std::string>();
    this->_h2_pushed_urls->reserve(Http2::push_diary_size);
  }

  if (_h2_pushed_urls->size() < Http2::push_diary_size) {
    _h2_pushed_urls->emplace(url);
  }
}

This means that it'll always search for the '\0' terminator when emplacing the string in _h2_pushed_urls. After checking the usage of add_url_to_pushed_table, it seems to be that the url_len should also be forwarded to the emplace. Is this correct observation or not?

freak82 avatar May 21 '24 08:05 freak82

Yes, it looks like a bug.

maskit avatar May 21 '24 15:05 maskit