From 6ccb760ea62a281acb5acfe15908b48b08fe898f Mon Sep 17 00:00:00 2001
From: Masakazu Kitajo <maskit@apache.org>
Date: Tue, 31 Jan 2023 13:34:16 -0700
Subject: [PATCH 1/3] Make Http3App class responsible for H3 transaction
 creation/deletion

---
 proxy/http3/Http3App.cc         |  2 +-
 proxy/http3/Http3Session.cc     |  4 +++-
 proxy/http3/Http3Transaction.cc | 10 ++++++----
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/proxy/http3/Http3App.cc b/proxy/http3/Http3App.cc
index 7519fe292f7..da9fa624a5b 100644
--- a/proxy/http3/Http3App.cc
+++ b/proxy/http3/Http3App.cc
@@ -351,9 +351,9 @@ Http3App::_handle_bidi_stream_on_write_complete(int event, VIO *vio)
     txn->handleEvent(event);
   }
   // FIXME There may be data to read
-  this->_ssn->remove_transaction(txn);
   this->_qc->stream_manager()->delete_stream(stream_id);
   this->_streams.erase(stream_id);
+  delete txn;
 }
 
 //
diff --git a/proxy/http3/Http3Session.cc b/proxy/http3/Http3Session.cc
index cf543c2bc45..217a75bca40 100644
--- a/proxy/http3/Http3Session.cc
+++ b/proxy/http3/Http3Session.cc
@@ -38,7 +38,10 @@ HQSession::HQSession(NetVConnection *vc) : ProxySession(vc)
 
 HQSession::~HQSession()
 {
+  // Transactions should be deleted first before HQSesson gets deleted.
+  // Delete remaining transactions just to prevent leaks.
   for (HQTransaction *t = this->_transaction_list.head; t; t = static_cast<HQTransaction *>(t->link.next)) {
+    ink_assert(t);
     delete t;
   }
 }
@@ -55,7 +58,6 @@ void
 HQSession::remove_transaction(HQTransaction *trans)
 {
   this->_transaction_list.remove(trans);
-  delete trans;
 
   return;
 }
diff --git a/proxy/http3/Http3Transaction.cc b/proxy/http3/Http3Transaction.cc
index 0916c91fc56..6c8e0cf9fd4 100644
--- a/proxy/http3/Http3Transaction.cc
+++ b/proxy/http3/Http3Transaction.cc
@@ -62,9 +62,14 @@ HQTransaction::HQTransaction(HQSession *session, QUICStreamVCAdapter::IOInfo &in
   this->_thread = this_ethread();
 
   this->_reader = this->_read_vio_buf.alloc_reader();
+
+  static_cast<HQSession *>(this->_proxy_ssn)->add_transaction(static_cast<HQTransaction *>(this));
 }
 
-HQTransaction::~HQTransaction() {}
+HQTransaction::~HQTransaction()
+{
+  static_cast<HQSession *>(this->_proxy_ssn)->remove_transaction(this);
+}
 
 void
 HQTransaction::set_active_timeout(ink_hrtime timeout_in)
@@ -295,7 +300,6 @@ HQTransaction::_signal_write_event()
 //
 Http3Transaction::Http3Transaction(Http3Session *session, QUICStreamVCAdapter::IOInfo &info) : super(session, info)
 {
-  static_cast<HQSession *>(this->_proxy_ssn)->add_transaction(static_cast<HQTransaction *>(this));
   QUICStreamId stream_id = this->_info.adapter.stream().id();
 
   this->_header_framer = new Http3HeaderFramer(this, &this->_write_vio, session->local_qpack(), stream_id);
@@ -513,8 +517,6 @@ Http3Transaction::has_request_body(int64_t content_length, bool is_chunked_set)
 //
 Http09Transaction::Http09Transaction(Http09Session *session, QUICStreamVCAdapter::IOInfo &info) : super(session, info)
 {
-  static_cast<HQSession *>(this->_proxy_ssn)->add_transaction(static_cast<HQTransaction *>(this));
-
   SET_HANDLER(&Http09Transaction::state_stream_open);
 }
 

From eb5ffd8aa664f44410af9271941aca534adf02df Mon Sep 17 00:00:00 2001
From: Masakazu Kitajo <maskit@apache.org>
Date: Tue, 7 Feb 2023 16:08:55 -0700
Subject: [PATCH 2/3] Fix use-after-free

---
 proxy/http3/Http3Session.cc | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/proxy/http3/Http3Session.cc b/proxy/http3/Http3Session.cc
index 217a75bca40..7cfeacd0713 100644
--- a/proxy/http3/Http3Session.cc
+++ b/proxy/http3/Http3Session.cc
@@ -40,9 +40,12 @@ HQSession::~HQSession()
 {
   // Transactions should be deleted first before HQSesson gets deleted.
   // Delete remaining transactions just to prevent leaks.
-  for (HQTransaction *t = this->_transaction_list.head; t; t = static_cast<HQTransaction *>(t->link.next)) {
+  HQTransaction *t = this->_transaction_list.head;
+  while (t) {
     ink_assert(t);
-    delete t;
+    auto x = t;
+    t      = static_cast<HQTransaction *>(t->link.next);
+    delete x;
   }
 }
 

From d80f90420f252eeb4ab48e66ba1302cde51b1988 Mon Sep 17 00:00:00 2001
From: Masakazu Kitajo <maskit@apache.org>
Date: Tue, 7 Feb 2023 16:16:15 -0700
Subject: [PATCH 3/3] Remove cleanup logic in HQSession destructor

---
 proxy/http3/Http3Session.cc | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/proxy/http3/Http3Session.cc b/proxy/http3/Http3Session.cc
index 7cfeacd0713..8a422fff599 100644
--- a/proxy/http3/Http3Session.cc
+++ b/proxy/http3/Http3Session.cc
@@ -39,14 +39,7 @@ HQSession::HQSession(NetVConnection *vc) : ProxySession(vc)
 HQSession::~HQSession()
 {
   // Transactions should be deleted first before HQSesson gets deleted.
-  // Delete remaining transactions just to prevent leaks.
-  HQTransaction *t = this->_transaction_list.head;
-  while (t) {
-    ink_assert(t);
-    auto x = t;
-    t      = static_cast<HQTransaction *>(t->link.next);
-    delete x;
-  }
+  ink_assert(this->_transaction_list.head);
 }
 
 void