http: fix datadog and squash handling of responses without body

docs/root/version_history/current.rst

@@ -55,6 +55,7 @@ Bug Fixes
 * fault: made the HeaderNameValues::prefix() method const.
 * grpc-web: fixed an issue with failing HTTP/2 requests on some browsers. Notably, WebKit-based browsers (https://bugs.webkit.org/show_bug.cgi?id=210108), Internet Explorer 11, and Edge (pre-Chromium).
 * http: fixed CVE-2020-25018 by rolling back the ``GURL`` dependency to previous state (reverted: ``2d69e30``, ``d828958``, and ``c9c4709`` commits) due to potential of crashing when Unicode URIs are present in requests.
+* http: fixed bugs in datadog and squash filter's handling of responses with no bodies.
 * http: made the HeaderValues::prefix() method const.
 * jwt_authn: supports jwt payload without "iss" field.
 * listener: fixed crash at listener inplace update when connetion load balancer is set.

source/extensions/filters/http/squash/squash_filter.cc

@@ -308,9 +308,7 @@ void SquashFilter::cleanup() {
 }
 
 Json::ObjectSharedPtr SquashFilter::getJsonBody(Http::ResponseMessagePtr&& m) {
-  Buffer::InstancePtr& data = m->body();
-  std::string jsonbody = data->toString();
-  return Json::Factory::loadFromString(jsonbody);
+  return Json::Factory::loadFromString(m->bodyAsString());
 }
 
 } // namespace Squash

source/extensions/tracers/datadog/datadog_tracer_impl.cc

@@ -138,7 +138,7 @@ void TraceReporter::onSuccess(const Http::AsyncClient::Request& request,
   } else {
     ENVOY_LOG(debug, "traces successfully submitted to datadog agent");
     driver_.tracerStats().reports_sent_.inc();
-    encoder_->handleResponse(http_response->body()->toString());
+    encoder_->handleResponse(http_response->bodyAsString());
   }
 }
 

test/extensions/filters/http/squash/squash_filter_test.cc

@@ -412,6 +412,19 @@ TEST_F(SquashFilterTest, InvalidJsonForGetAttachment) {
   completeRequest("200", "This is not a JSON object");
 }
 
+TEST_F(SquashFilterTest, InvalidResponseWithNoBody) {
+  doDownstreamRequest();
+  // Expect the get attachment request
+  expectAsyncClientSend();
+  completeCreateRequest();
+
+  auto retry_timer = new NiceMock<Envoy::Event::MockTimer>(&filter_callbacks_.dispatcher_);
+  EXPECT_CALL(*retry_timer, enableTimer(config_->attachmentPollPeriod(), _));
+  Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl(Http::ResponseHeaderMapPtr{
+      new Http::TestResponseHeaderMapImpl{{":status", "200"}, {"content-length", "0"}}}));
+  popPendingCallback()->onSuccess(request_, std::move(msg));
+}
+
 TEST_F(SquashFilterTest, DestroyedInFlight) {
   doDownstreamRequest();
 

test/extensions/tracers/datadog/datadog_tracer_impl_test.cc

@@ -185,6 +185,47 @@ TEST_F(DatadogDriverTest, FlushSpansTimer) {
   EXPECT_EQ(0U, stats_.counter("tracing.datadog.reports_failed").value());
 }
 
+TEST_F(DatadogDriverTest, NoBody) {
+  setupValidDriver();
+
+  Http::MockAsyncClientRequest request(&cm_.async_client_);
+  Http::AsyncClient::Callbacks* callback;
+  const absl::optional<std::chrono::milliseconds> timeout(std::chrono::seconds(1));
+  EXPECT_CALL(cm_.async_client_,
+              send_(_, _, Http::AsyncClient::RequestOptions().setTimeout(timeout)))
+      .WillOnce(
+          Invoke([&](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks& callbacks,
+                     const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* {
+            callback = &callbacks;
+
+            EXPECT_EQ("fake_cluster", message->headers().getHostValue());
+            EXPECT_EQ("application/msgpack", message->headers().getContentTypeValue());
+
+            return &request;
+          }));
+
+  Tracing::SpanPtr span = driver_->startSpan(config_, request_headers_, operation_name_,
+                                             start_time_, {Tracing::Reason::Sampling, true});
+  span->finishSpan();
+
+  // Timer should be re-enabled.
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(900), _));
+
+  timer_->invokeCallback();
+
+  EXPECT_EQ(1U, stats_.counter("tracing.datadog.timer_flushed").value());
+  EXPECT_EQ(1U, stats_.counter("tracing.datadog.traces_sent").value());
+
+  Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl(Http::ResponseHeaderMapPtr{
+      new Http::TestResponseHeaderMapImpl{{":status", "200"}, {"content-length", "0"}}}));
+  callback->onSuccess(request, std::move(msg));
+
+  EXPECT_EQ(0U, stats_.counter("tracing.datadog.reports_skipped_no_cluster").value());
+  EXPECT_EQ(1U, stats_.counter("tracing.datadog.reports_sent").value());
+  EXPECT_EQ(0U, stats_.counter("tracing.datadog.reports_dropped").value());
+  EXPECT_EQ(0U, stats_.counter("tracing.datadog.reports_failed").value());
+}
+
 TEST_F(DatadogDriverTest, SkipReportIfCollectorClusterHasBeenRemoved) {
   Upstream::ClusterUpdateCallbacks* cluster_update_callbacks;
   EXPECT_CALL(cm_, addThreadLocalClusterUpdateCallbacks_(_))