TS-4697: free MIOBuffer if fails on ipallow check.

iocore/net/I_SessionAccept.h

@@ -29,12 +29,40 @@
 
 struct AclRecord;
 
+/**
+   The base class SessionAccept can not be used directly.
+   The inherited class of SessionAccept (ex. HttpSessionAccept) is designed to
+
+     - Check IPAllow policy
+     - Create ClientSession
+     - Pass NetVC and MIOBuffer by call ClientSession::new_connection()
+
+   NULL mutex:
+
+     - One specific protocol has ONLY one inherited class of SessionAccept.
+
+     - The object of this class is shared by all incoming request / NetVC that
+       identified as the protocol by ProtocolSessionProbe.
+
+     - The inherited class of SessionAccept is non-blocked to allow parellel accepts
+
+   To implement a inherited class of SessionAccept:
+
+     - No state is recorded by the handler
+
+     - Values are required to be set during construction and never changed
+
+     - Can not put into EventSystem.
+
+   So a NULL mutex is safe to continuation.
+*/
+
 class SessionAccept : public Continuation
 {
 public:
   SessionAccept(ProxyMutex *amutex) : Continuation(amutex) { SET_HANDLER(&SessionAccept::mainEvent); }
   ~SessionAccept() {}
-  virtual void accept(NetVConnection *, MIOBuffer *, IOBufferReader *) = 0;
+  virtual bool accept(NetVConnection *, MIOBuffer *, IOBufferReader *) = 0;
 
   /* Returns NULL if the specified client_ip is not allowed by ip_allow
    * Returns a pointer to the relevant IP policy for later processing otherwise */

iocore/net/P_SSLNextProtocolAccept.h

@@ -37,7 +37,7 @@ class SSLNextProtocolAccept : public SessionAccept
   SSLNextProtocolAccept(Continuation *, bool);
   ~SSLNextProtocolAccept();
 
-  void accept(NetVConnection *, MIOBuffer *, IOBufferReader *);
+  bool accept(NetVConnection *, MIOBuffer *, IOBufferReader *);
 
   // Register handler as an endpoint for the specified protocol. Neither
   // handler nor protocol are copied, so the caller must guarantee their

iocore/net/SSLNextProtocolAccept.cc

@@ -145,10 +145,11 @@ SSLNextProtocolAccept::mainEvent(int event, void *edata)
   }
 }
 
-void
+bool
 SSLNextProtocolAccept::accept(NetVConnection *, MIOBuffer *, IOBufferReader *)
 {
   ink_release_assert(0);
+  return false;
 }
 
 bool

proxy/ProtocolProbeSessionAccept.cc

@@ -76,7 +76,6 @@ struct ProtocolProbeTrampoline : public Continuation, public ProtocolProbeSessio
     case VC_EVENT_ACTIVE_TIMEOUT:
     case VC_EVENT_INACTIVITY_TIMEOUT:
       // Error ....
-      netvc->do_io_close();
       goto done;
     case VC_EVENT_READ_READY:
     case VC_EVENT_READ_COMPLETE:
@@ -89,7 +88,6 @@ struct ProtocolProbeTrampoline : public Continuation, public ProtocolProbeSessio
 
     if (!reader->is_read_avail_more_than(minimum_read_size - 1)) {
       // Not enough data read. Well, that sucks.
-      netvc->do_io_close();
       goto done;
     }
 
@@ -103,16 +101,19 @@ struct ProtocolProbeTrampoline : public Continuation, public ProtocolProbeSessio
 
     if (probeParent->endpoint[key] == NULL) {
       Warning("Unregistered protocol type %d", key);
-      netvc->do_io_close();
       goto done;
     }
 
     // Directly invoke the session acceptor, letting it take ownership of the input buffer.
-    probeParent->endpoint[key]->accept(netvc, this->iobuf, reader);
+    if (!probeParent->endpoint[key]->accept(netvc, this->iobuf, reader)) {
+      // IPAllow check fails in XxxSessionAccept::accept() if false returned.
+      goto done;
+    }
     delete this;
     return EVENT_CONT;
 
   done:
+    netvc->do_io_close();
     free_MIOBuffer(this->iobuf);
     this->iobuf = NULL;
     delete this;
@@ -151,10 +152,11 @@ ProtocolProbeSessionAccept::mainEvent(int event, void *data)
   return EVENT_CONT;
 }
 
-void
+bool
 ProtocolProbeSessionAccept::accept(NetVConnection *, MIOBuffer *, IOBufferReader *)
 {
   ink_release_assert(0);
+  return false;
 }
 
 void

proxy/ProtocolProbeSessionAccept.h

@@ -48,7 +48,7 @@ class ProtocolProbeSessionAccept : public SessionAccept, public ProtocolProbeSes
   ~ProtocolProbeSessionAccept() {}
   void registerEndpoint(ProtoGroupKey key, SessionAccept *ap);
 
-  void accept(NetVConnection *, MIOBuffer *, IOBufferReader *);
+  bool accept(NetVConnection *, MIOBuffer *, IOBufferReader *);
 
 private:
   int mainEvent(int event, void *netvc);

proxy/http/HttpSessionAccept.cc

@@ -27,7 +27,7 @@
 #include "I_Machine.h"
 #include "Error.h"
 
-void
+bool
 HttpSessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferReader *reader)
 {
   sockaddr const *client_ip   = netvc->get_remote_addr();
@@ -45,8 +45,7 @@ HttpSessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferReade
       // if client address forbidden, close immediately //
       ////////////////////////////////////////////////////
       Warning("client '%s' prohibited by ip-allow policy", ats_ip_ntop(client_ip, ipb, sizeof(ipb)));
-      netvc->do_io_close();
-      return;
+      return false;
     }
   }
 
@@ -73,17 +72,21 @@ HttpSessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferReade
 
   new_session->new_connection(netvc, iobuf, reader, backdoor);
 
-  return;
+  return true;
 }
 
 int
 HttpSessionAccept::mainEvent(int event, void *data)
 {
+  NetVConnection *netvc;
   ink_release_assert(event == NET_EVENT_ACCEPT || event == EVENT_ERROR);
   ink_release_assert((event == NET_EVENT_ACCEPT) ? (data != 0) : (1));
 
   if (event == NET_EVENT_ACCEPT) {
-    this->accept(static_cast<NetVConnection *>(data), NULL, NULL);
+    netvc = static_cast<NetVConnection *>(data);
+    if (!this->accept(netvc, NULL, NULL)) {
+      netvc->do_io_close();
+    }
     return EVENT_CONT;
   }
 

proxy/http/HttpSessionAccept.h

@@ -205,7 +205,7 @@ class HttpSessionAccept : public SessionAccept, private detail::HttpSessionAccep
   }
 
   ~HttpSessionAccept() { return; }
-  void accept(NetVConnection *, MIOBuffer *, IOBufferReader *);
+  bool accept(NetVConnection *, MIOBuffer *, IOBufferReader *);
   int mainEvent(int event, void *netvc);
 
 private:

proxy/http2/Http2SessionAccept.cc

@@ -36,16 +36,15 @@ Http2SessionAccept::~Http2SessionAccept()
 {
 }
 
-void
+bool
 Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferReader *reader)
 {
   sockaddr const *client_ip           = netvc->get_remote_addr();
   const AclRecord *session_acl_record = testIpAllowPolicy(client_ip);
   if (!session_acl_record) {
     ip_port_text_buffer ipb;
     Warning("HTTP/2 client '%s' prohibited by ip-allow policy", ats_ip_ntop(client_ip, ipb, sizeof(ipb)));
-    netvc->do_io_close();
-    return;
+    return false;
   }
   netvc->attributes = this->options.transport_type;
 
@@ -59,16 +58,22 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
   Http2ClientSession *new_session = THREAD_ALLOC_INIT(http2ClientSessionAllocator, this_ethread());
   new_session->acl_record         = session_acl_record;
   new_session->new_connection(netvc, iobuf, reader, false /* backdoor */);
+
+  return true;
 }
 
 int
 Http2SessionAccept::mainEvent(int event, void *data)
 {
+  NetVConnection *netvc;
   ink_release_assert(event == NET_EVENT_ACCEPT || event == EVENT_ERROR);
   ink_release_assert((event == NET_EVENT_ACCEPT) ? (data != 0) : (1));
 
   if (event == NET_EVENT_ACCEPT) {
-    this->accept(static_cast<NetVConnection *>(data), NULL, NULL);
+    netvc = static_cast<NetVConnection *>(data);
+    if (!this->accept(netvc, NULL, NULL)) {
+      netvc->do_io_close();
+    }
     return EVENT_CONT;
   }
 

proxy/http2/Http2SessionAccept.h

@@ -43,7 +43,7 @@ struct Http2SessionAccept : public SessionAccept {
   explicit Http2SessionAccept(const HttpSessionAccept::Options &);
   ~Http2SessionAccept();
 
-  void accept(NetVConnection *, MIOBuffer *, IOBufferReader *);
+  bool accept(NetVConnection *, MIOBuffer *, IOBufferReader *);
   int mainEvent(int event, void *netvc);
 
 private: