apache / trafficserver

Apache Traffic Serverâ„¢ is a fast, scalable and extensible HTTP/1.1 and HTTP/2 compliant caching proxy server.
https://trafficserver.apache.org/
Apache License 2.0
1.82k stars 804 forks source link

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

Closed freak82 closed 5 months ago

freak82 commented 6 months ago

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?

maskit commented 6 months ago

Yes, it looks like a bug.