proxygen: clear FilterChain tail_ in release()

Summary:
`FilterChain::release()` hands the chain head to the caller but leaves `tail_`
pointing at the last filter. Heap allocated filters delete themselves as soon as
they read EOM or an error, and `FilterChain::setSource()` routes through `tail_`
whenever a filter was inserted - so a released chain can dereference a freed
filter.

`HTTPSourceReader::read()` releases the chain up front and
`HTTPClient::makeRequestReadResponse()` then calls `reader.setSource(nullptr)`,
which makes any heap allocated filter added via
`HTTPSourceReader::insertFilter()` a use-after-free at the end of every request.
That is the ownership model every `FilterFactory` produces
(`RequestContextFilterFactory`, `StatsFilterUtil`, `CompressionFilterFactory`,
`ClientDecompressionFilterFactory` all `setHeapAllocated()`), so today
`FilterFactory` filters cannot be used with an `HTTPSourceReader` at all. The
existing reader filter users - the redirect handler and the logger - own their
filters as members, which is why this has not fired.

Clearing `tail_` makes a released chain empty, so a later `setSource()` applies
to the head instead of a filter the chain no longer owns. In the normal path
that turns the trailing `setSource(nullptr)` into a no-op, since every filter
has already cleared its own source by then.

Split out of D114432220, which needs this to install `FilterFactory` filters on
an `HTTPSourceReader`.

Reviewed By: hanidamlaj

Differential Revision: D114950502

fbshipit-source-id: e334deb011355f5cae8cb9c38b61f15a3f659913

Author: Sujay Patel

proxygen/lib/http/coro/HTTPSourceFilterChain.h

@@ -66,6 +66,7 @@ class FilterChain {
 
   // release ownership of chain
   HTTPSource* release() {
+    tail_ = nullptr;
     return head_.release();
   }
 

proxygen/lib/http/coro/test/HTTPSourceTests.cpp

@@ -14,6 +14,7 @@
 #include "proxygen/lib/http/coro/HTTPSourceReader.h"
 #include "proxygen/lib/http/coro/HTTPStreamSource.h"
 #include "proxygen/lib/http/coro/HTTPStreamSourceHolder.h"
+#include "proxygen/lib/http/coro/filters/MutateFilter.h"
 #include "proxygen/lib/http/coro/test/Mocks.h"
 #include "proxygen/lib/http/coro/util/ExecutorSourceFilter.h"
 #include "proxygen/lib/http/coro/util/test/TestHelpers.h"
@@ -1269,6 +1270,27 @@ TEST(HTTPSourceReader, filter) {
   EXPECT_EQ(events, 2);
 }
 
+// A heap allocated filter deletes itself when it reads EOM, so the reader must
+// not touch it after the read - notably HTTPClient clears the reader's source
+// once the read completes.
+TEST(HTTPSourceReader, heapAllocatedFilter) {
+  folly::EventBase evb;
+  bool sawHeaders = false;
+  auto* filter = new MutateFilter(
+      /*source=*/nullptr, [&sawHeaders](HTTPHeaderEvent& headerEvent) {
+        sawHeaders = true;
+        EXPECT_EQ(headerEvent.headers->getStatusCode(), 200);
+      });
+  filter->setHeapAllocated();
+
+  HTTPSourceReader reader;
+  reader.insertFilter(filter);
+  reader.setSource(HTTPFixedSource::makeFixedResponse(200, makeBuf(100)));
+  folly::coro::blockingWait(reader.read(), &evb);
+  reader.setSource(nullptr);
+  EXPECT_TRUE(sawHeaders);
+}
+
 TEST(HTTPErrorTests, ErrorStrings) {
   EXPECT_EQ(getErrorString(HTTPErrorCode::PROTOCOL_ERROR), "PROTOCOL_ERROR");
   EXPECT_EQ(getErrorString(HTTPErrorCode::QPACK_DECOMPRESSION_FAILED),