From: Christian Hofstaedtler <christian@hofstaedtler.name>
Date: Sat, 5 Oct 2013 17:16:17 +0000 (+0200)
Subject: ws: rework exception/error handling
X-Git-Tag: rec-3.6.0-rc1~416^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=331969453f1d2f39dbd8980fd82e08ccb7803f10;p=pdns

ws: rework exception/error handling

HTTP-level errors now cause HttpExceptions, and generate corresponding
HTTP status replies. Those replies can be served as HTML, JSON or plain
text, depending on what the client wanted.

Also, this gets rid of the old "class Exception", and duplicate catch
statements, because SessionException is a PDNSException already.
---

diff --git a/pdns/session.cc b/pdns/session.cc
index 0e338d35a..716be607d 100644
--- a/pdns/session.cc
+++ b/pdns/session.cc
@@ -291,7 +291,7 @@ Server::Server(int port, const string &localaddress)
   s = socket(d_local.sin4.sin_family ,SOCK_STREAM,0);
 
   if(s < 0)
-    throw Exception(string("socket: ")+strerror(errno));
+    throw SessionException(string("socket: ")+strerror(errno));
 
   Utility::setCloseOnExec(s);
   
diff --git a/pdns/session.hh b/pdns/session.hh
index 998876017..c2902520a 100644
--- a/pdns/session.hh
+++ b/pdns/session.hh
@@ -103,12 +103,4 @@ private:
   int s;
 };
 
-class Exception
-{
-public:
-  Exception(){reason="Unspecified";};
-  Exception(string r){reason=r;};
-  string reason;
-};
-
 #endif /* SESSION_HH */
diff --git a/pdns/webserver.cc b/pdns/webserver.cc
index 79a6cd314..8e0d8f693 100644
--- a/pdns/webserver.cc
+++ b/pdns/webserver.cc
@@ -25,6 +25,7 @@
 #include <stdio.h>
 #include "dns.hh"
 #include "base64.hh"
+#include "json.hh"
 
 
 map<string,WebServer::HandlerFunction *>WebServer::d_functions;
@@ -47,16 +48,19 @@ void WebServer::setCaller(void *that)
 }
 
 void *WebServer::serveConnection(void *p)
-{
+try {
   pthread_detach(pthread_self());
   Session *client=static_cast<Session *>(p);
+  bool want_html=false;
+  bool want_json=false;
+
   try {
     string line;
     client->setTimeout(5);
     client->getLine(line);
     stripLine(line);
     if(line.empty())
-      throw Exception("Invalid web request");
+      throw HttpBadRequestException();
     //    L<<"page: "<<line<<endl;
 
     vector<string> parts;
@@ -110,27 +114,44 @@ void *WebServer::serveConnection(void *p)
       client->getLine(line);
       stripLine(line);
 
-      //      L<<Logger::Error<<"got line: '"<<line<<"'"<<endl;
-      if(!toLower(line).find("authorization: basic ")) {
-        string cookie=line.substr(21);
+      if(line.empty())
+	break;
+
+      size_t colon = line.find(":");
+      if(colon==std::string::npos)
+	throw HttpBadRequestException();
+
+      string header = toLower(line.substr(0, colon));
+      string value = line.substr(line.find_first_not_of(' ', colon+1));
+
+      if(header == "authorization" && toLower(value).find("basic ") == 0) {
+        string cookie=value.substr(6);
         string plain;
 
         B64Decode(cookie,plain);
         vector<string>cparts;
         stringtok(cparts,plain,":");
-        //	L<<Logger::Error<<"Entered password: '"<<cparts[1].c_str()<<"', should be '"<<d_password.c_str()<<"'"<<endl;
+        // L<<Logger::Error<<"Entered password: '"<<cparts[1].c_str()<<"', should be '"<<d_password.c_str()<<"'"<<endl;
         if(cparts.size()==2 && !strcmp(cparts[1].c_str(),d_password.c_str())) { // this gets rid of terminating zeros
           authOK=1;
         }
       }
-      else if(boost::starts_with(line, "Content-Length: ") && method=="POST") {
-	postlen = atoi(line.c_str() + strlen("Content-Length: "));
+      else if(header == "content-length" && method=="POST") {
+	postlen = atoi(value.c_str());
 //	cout<<"Got a post: "<<postlen<<" bytes"<<endl;
       }
+      else if(header == "accept") {
+	// json wins over html
+	if(value.find("application/json")!=std::string::npos) {
+	  want_json=true;
+	} else if(value.find("text/html")!=std::string::npos) {
+	  want_html=true;
+	}
+      }
       else
 	; // cerr<<"Ignoring line: "<<line<<endl;
       
-    }while(!line.empty());
+    } while(true);
 
     string post;
     if(postlen) 
@@ -138,17 +159,8 @@ void *WebServer::serveConnection(void *p)
   
  //   cout<<"Post: '"<<post<<"'"<<endl;
 
-    if(!d_password.empty() && !authOK) {
-      client->putLine("HTTP/1.1 401 OK\n");
-      client->putLine("WWW-Authenticate: Basic realm=\"PowerDNS\"\n");
-      
-      client->putLine("Connection: close\n");
-      client->putLine("Content-Type: text/html; charset=utf-8\n\n");
-      client->putLine("Please enter a valid password!\n");
-      client->close();
-      delete client;
-      return 0;
-    }
+    if(!d_password.empty() && !authOK)
+      throw HttpUnauthorizedException();
 
     HandlerFunction *fptr;
     if(d_functions.count(baseUrl) && (fptr=d_functions[baseUrl])) {
@@ -163,44 +175,45 @@ void *WebServer::serveConnection(void *p)
       client->putLine(ret);
     }
     else {
-      client->putLine("HTTP/1.1 404 Not found\n");
-      client->putLine("Connection: close\n");
-      client->putLine("Content-Type: text/html; charset=utf-8\n\n");
-      // FIXME: CSS problem?
-      client->putLine("<html><body><h1>Did not find file '"+baseUrl+"'</body></html>\n");
+      throw HttpNotFoundException();
     }
-        
-    client->close();
-    delete client;
-    client=0;
-    return 0;
 
   }
-  catch(SessionTimeoutException &e) {
-    // L<<Logger::Error<<"Timeout in webserver"<<endl;
-  }
-  catch(SessionException &e) {
-    L<<Logger::Error<<"Fatal error in webserver: "<<e.reason<<endl;
-  }
-  catch(Exception &e) {
-    L<<Logger::Error<<"Exception in webserver: "<<e.reason<<endl;
-  }
-  catch(PDNSException &e) {
-    L<<Logger::Error<<"Exception in webserver: "<<e.reason<<endl;
-  }
-  catch(std::exception &e) {
-    L<<Logger::Error<<"STL Exception in webserver: "<<e.what()<<endl;
-  }
-  catch(...) {
-    L<<Logger::Error<<"Unknown exception in webserver"<<endl;
-  }
-  if(client) {
-    client->close();
-    delete client;
-    client=0;
+  catch(HttpException &e) {
+    client->putLine(e.statusLine());
+    client->putLine("Connection: close\n");
+    client->putLine(e.headers());
+    if(want_html) {
+      client->putLine("Content-Type: text/html; charset=utf-8\n\n");
+      client->putLine("<!html><title>" + e.what() + "</title><h1>" + e.what() + "</h1>");
+    } else if (want_json) {
+      client->putLine("Content-Type: application/json\n\n");
+      client->putLine(returnJSONError(e.what()));
+    } else {
+      client->putLine("Content-Type: text/plain; charset=utf-8\n\n");
+      client->putLine(e.what());
+    }
   }
+
+  client->close();
+  delete client;
+  client=0;
+
   return 0;
 }
+catch(SessionTimeoutException &e) {
+  // L<<Logger::Error<<"Timeout in webserver"<<endl;
+}
+catch(PDNSException &e) {
+  L<<Logger::Error<<"Exception in webserver: "<<e.reason<<endl;
+}
+catch(std::exception &e) {
+  L<<Logger::Error<<"STL Exception in webserver: "<<e.what()<<endl;
+}
+catch(...) {
+  L<<Logger::Error<<"Unknown exception in webserver"<<endl;
+}
+
 
 WebServer::WebServer(const string &listenaddress, int port, const string &password)
 {
@@ -233,12 +246,6 @@ void WebServer::go()
   catch(SessionTimeoutException &e) {
     //    L<<Logger::Error<<"Timeout in webserver"<<endl;
   }
-  catch(SessionException &e) {
-    L<<Logger::Error<<"Fatal error in webserver: "<<e.reason<<endl;
-  }
-  catch(Exception &e) {
-    L<<Logger::Error<<"Fatal error in main webserver thread: "<<e.reason<<endl;
-  }
   catch(PDNSException &e) {
     L<<Logger::Error<<"Exception in main webserver thread: "<<e.reason<<endl;
   }
diff --git a/pdns/webserver.hh b/pdns/webserver.hh
index bde1e7634..0d08dd1a8 100644
--- a/pdns/webserver.hh
+++ b/pdns/webserver.hh
@@ -25,6 +25,55 @@
 #include "namespaces.hh"
 class Server;
 
+class HttpException
+{
+public:
+  HttpException(int status_code, const std::string& reason_phrase) :
+    d_status_code(status_code), d_reason_phrase(reason_phrase)
+  {
+  };
+
+  virtual std::string statusLine() const {
+    return "HTTP/1.0 " + lexical_cast<string>(d_status_code) + " " + d_reason_phrase + "\n";
+  }
+
+  virtual std::string headers() const {
+    return "";
+  }
+
+  virtual std::string what() const {
+    return d_reason_phrase;
+  }
+
+private:
+  int d_status_code;
+  std::string d_reason_phrase;
+};
+
+class HttpBadRequestException : public HttpException {
+public:
+  HttpBadRequestException() : HttpException(400, "Bad Request") { };
+};
+
+class HttpUnauthorizedException : public HttpException {
+public:
+  HttpUnauthorizedException() : HttpException(401, "Unauthorized") { };
+
+  std::string headers() const {
+    return "WWW-Authenticate: Basic realm=\"PowerDNS\"\n";
+  }
+};
+
+class HttpNotFoundException : public HttpException {
+public:
+  HttpNotFoundException() : HttpException(404, "Not Found") { };
+};
+
+class HttpMethodNotAllowedException : public HttpException {
+public:
+  HttpMethodNotAllowedException() : HttpException(405, "Method Not Allowed") { };
+};
+
 class WebServer
 {
 public:
diff --git a/pdns/ws.cc b/pdns/ws.cc
index d7ea5cebb..4b17211f8 100644
--- a/pdns/ws.cc
+++ b/pdns/ws.cc
@@ -273,7 +273,7 @@ static int intFromJson(const Value& val) {
   } else if (val.IsString()) {
     return atoi(val.GetString());
   } else {
-    throw Exception("Value not an Integer");
+    throw PDNSException("Value not an Integer");
   }
 }
 
@@ -429,9 +429,8 @@ static string jsonDispatch(const string& method, const string& post, varmap_t& v
     return returnJSONObject(object);
   }
   else if(command == "pdns-control") {
-    // TODO: turn this into a 405
     if(method!="POST")
-      return returnJSONError("pdns-control requires a POST");
+      throw HttpMethodNotAllowedException();
     // cout<<"post: "<<post<<endl;
     rapidjson::Document document;
     if(document.Parse<0>(post.c_str()).HasParseError())
@@ -560,8 +559,7 @@ static string jsonDispatch(const string& method, const string& post, varmap_t& v
       map<string, string> success; // empty success object
       return returnJSONObject(success);
     } else {
-      // TODO: turn this into a 405
-      return returnJSONError("Method not allowed");
+      throw HttpMethodNotAllowedException();
     }
   }
   else if(command=="log-grep") {