replace Server::shouldHandle() with a std::function

Change the Server API so that users don't have to subclass it in order
to override shouldHandle().  Now users can specify a custom
std::function for checking allowed URLs.

This will make it easier to provide alternative Server implementations.
Previously InternalPageServerImpl had to subclass LibEventServer in
order to override shouldHandle().  This tied the code to LibEventServer,
so that it could not be easily changed to an alternative implementation.

It seems like this URL checking functionality should perhaps be moved
into the RequestHandler.  However, for now this is a simpler change.

hphp/runtime/base/server/satellite_server.cpp

@@ -70,46 +70,33 @@ SatelliteServerInfo::SatelliteServerInfo(Hdf hdf) {
   }
 }
 
+bool SatelliteServerInfo::checkMainURL(const std::string& path) {
+  String url(path.c_str(), path.size(), AttachLiteral);
+  for (std::set<string>::const_iterator iter =
+         SatelliteServerInfo::InternalURLs.begin();
+       iter != SatelliteServerInfo::InternalURLs.end(); ++iter) {
+    Variant ret = preg_match
+      (String(iter->c_str(), iter->size(), AttachLiteral), url);
+    if (ret.toInt64() > 0) {
+      return false;
+    }
+  }
+  return true;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // InternalPageServer: LibEventServer + allowed URL checking
 
-DECLARE_BOOST_TYPES(InternalPageServerImpl);
-class InternalPageServerImpl : public LibEventServer {
-public:
-  InternalPageServerImpl(const std::string &address, int port, int thread,
-                         int timeoutSeconds) :
-    LibEventServer(address, port, thread, timeoutSeconds) {
-  }
-  void create(const std::set<std::string> &urls) {
-    m_allowedURLs = urls;
-  }
-
-  virtual bool shouldHandle(const std::string &cmd) {
-    String url(cmd.c_str(), cmd.size(), AttachLiteral);
-    for (set<string>::const_iterator iter = m_allowedURLs.begin();
-         iter != m_allowedURLs.end(); ++iter) {
-      Variant ret = preg_match
-        (String(iter->c_str(), iter->size(), AttachLiteral), url);
-      if (ret.toInt64() > 0) {
-        return true;
-      }
-    }
-    return false;
-  }
-
-private:
-  std::set<std::string> m_allowedURLs;
-};
-
 class InternalPageServer : public SatelliteServer {
 public:
-  explicit InternalPageServer(SatelliteServerInfoPtr info) {
-    auto const server = boost::make_shared<InternalPageServerImpl>(
+  explicit InternalPageServer(SatelliteServerInfoPtr info)
+    : m_allowedURLs(info->getURLs()) {
+    m_server = boost::make_shared<LibEventServer>(
         RuntimeOption::ServerIP, info->getPort(), info->getThreadCount(),
         info->getTimeoutSeconds());
-    server->setRequestHandlerFactory<HttpRequestHandler>();
-    server->create(info->getURLs());
-    m_server = server;
+    m_server->setRequestHandlerFactory<HttpRequestHandler>();
+    m_server->setUrlChecker(std::bind(&InternalPageServer::checkURL, this,
+                                      std::placeholders::_1));
   }
 
   virtual void start() {
@@ -119,8 +106,22 @@ public:
     m_server->stop();
     m_server->waitForEnd();
   }
+
 private:
+  bool checkURL(const std::string &path) const {
+    String url(path.c_str(), path.size(), AttachLiteral);
+    for (const auto &allowed : m_allowedURLs) {
+      Variant ret = preg_match
+        (String(allowed.c_str(), allowed.size(), AttachLiteral), url);
+      if (ret.toInt64() > 0) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   ServerPtr m_server;
+  std::set<std::string> m_allowedURLs;
 };
 
 ///////////////////////////////////////////////////////////////////////////////

hphp/runtime/base/server/satellite_server.h

@@ -62,8 +62,13 @@ public:
   static std::set<std::string> InternalURLs;
   static int DanglingServerPort;
 
+  /**
+   * Check whether a requested path should be allowed on the main server.
+   */
+  static bool checkMainURL(const std::string& path);
+
 public:
-  SatelliteServerInfo(Hdf hdf);
+  explicit SatelliteServerInfo(Hdf hdf);
 
   const std::string &getName() const { return m_name;}
   SatelliteServer::Type getType() const { return m_type;}

hphp/runtime/base/server/server.cpp

@@ -14,8 +14,8 @@
    +----------------------------------------------------------------------+
 */
 
-#include "hphp/runtime/base/complex_types.h"
 #include "hphp/runtime/base/server/server.h"
+#include "hphp/runtime/base/complex_types.h"
 #include "hphp/runtime/base/server/satellite_server.h"
 #include "hphp/runtime/base/preg.h"
 #include <signal.h>
@@ -50,22 +50,9 @@ void Server::InstallStopSignalHandlers(ServerPtr server) {
 
 Server::Server(const std::string &address, int port, int threadCount)
   : m_address(address), m_port(port), m_threadCount(threadCount),
+    m_urlChecker(SatelliteServerInfo::checkMainURL),
     m_status(NOT_YET_STARTED) {
 }
 
-bool Server::shouldHandle(const std::string &cmd) {
-  String url(cmd.c_str(), cmd.size(), AttachLiteral);
-  for (std::set<string>::const_iterator iter =
-         SatelliteServerInfo::InternalURLs.begin();
-       iter != SatelliteServerInfo::InternalURLs.end(); ++iter) {
-    Variant ret = preg_match
-      (String(iter->c_str(), iter->size(), AttachLiteral), url);
-    if (ret.toInt64() > 0) {
-      return false;
-    }
-  }
-  return true;
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 }

hphp/runtime/base/server/server.h

@@ -86,6 +86,7 @@ public:
 };
 
 typedef std::function<std::unique_ptr<RequestHandler>()> RequestHandlerFactory;
+typedef std::function<bool(const std::string&)> URLChecker;
 
 /**
  * Base class of an HTTP server. Defining minimal interface an HTTP server
@@ -135,6 +136,16 @@ public:
     });
   }
 
+  /**
+   * Set the URLChecker function which determines which paths this server is
+   * allowed to server.
+   *
+   * Defaults to SatelliteServerInfo::checkURL()
+   */
+  void setUrlChecker(const URLChecker& checker) {
+    m_urlChecker = checker;
+  }
+
   /**
    * Informational.
    */
@@ -192,9 +203,11 @@ public:
   }
 
   /**
-   * Overwrite for URL blocking.
+   * Check whether a request to the specified server path is allowed.
    */
-  virtual bool shouldHandle(const std::string &cmd);
+  bool shouldHandle(const std::string &path) {
+    return m_urlChecker(path);
+  }
 
   /**
    * To enable SSL of the current server, it will listen to an additional
@@ -208,6 +221,7 @@ protected:
   int m_threadCount;
   mutable Mutex m_mutex;
   RequestHandlerFactory m_handlerFactory;
+  URLChecker m_urlChecker;
 
 private:
   RunStatus m_status;