Some refactoring on the HTTP codebase

luprex/core/cpp/http.cpp

@@ -644,6 +644,8 @@ void HttpClientRequestMap::deserialize(StreamBuffer *sb) {
 }
 
 HttpParser::HttpParser() {
+    request_id_ = 0;
+    is_request_ = false;
     status_ = 0;
     mime_type_ = "";
     content_length_ = -1;
@@ -959,7 +961,7 @@ bool HttpParser::parse_content(std::string_view &view, bool closed) {
     return true;
 }
 
-void HttpParser::store_parsed(LuaStack &LS0, LuaSlot tab) const {
+void HttpParser::store(LuaStack &LS0, LuaSlot tab) const {
     LuaVar ptab;
     LuaStack LS(LS0.state(), ptab);
 
@@ -1002,7 +1004,11 @@ void HttpParser::store_parsed(LuaStack &LS0, LuaSlot tab) const {
     }
 }
 
-void HttpParser::parser_generate_debug_string(std::ostream &oss) const {
+eng::string HttpParser::debug_string() const {
+    eng::ostringstream oss;
+    if (request_id_ != 0) {
+        oss << "  request_id: " << request_id_ << std::endl;
+    }
     oss << "  status_code: " << status_ << std::endl;
     oss << "  error: " << error_ << std::endl;
     if (content_length_ >= 0) {
@@ -1035,6 +1041,7 @@ void HttpParser::parser_generate_debug_string(std::ostream &oss) const {
     if (comm_length_ > 0) {
         oss << "  comm_length: " << comm_length_ << std::endl;
     }
+    return oss.str();
 }
 
 void HttpParser::clear_content_on_error() {
@@ -1045,23 +1052,9 @@ void HttpParser::clear_content_on_error() {
     }
 }
 
-HttpClientResponse::HttpClientResponse() {
+void HttpParser::parse_response(std::string_view view, bool closed) {
-    request_id_ = 0;
-    is_request_ = false;
-}
-
-eng::string HttpClientResponse::DebugString() const {
-    eng::ostringstream oss;
-    oss << "HttpClientResponse:" << std::endl;
-    if (request_id_ != 0) {
-        oss << "  request_id: " << request_id_ << std::endl;
-    }
-    parser_generate_debug_string(oss);
-    return oss.str();
-}
-
-void HttpClientResponse::parse(std::string_view view, bool closed) {
     std::string_view original_view = view;
+    is_request_ = false;
 
     // Parse the status line.
     if (!parse_status_line(view, closed)) {
@@ -1079,7 +1072,7 @@ void HttpClientResponse::parse(std::string_view view, bool closed) {
     }
 
     // Calculate the response length.
-    set_comm_length(original_view.size() - view.size());
+    comm_length_ = original_view.size() - view.size();
 
     // If it's not a redirect, ignore location.
     if ((status_ < 300) || (status_ > 399)) {
@@ -1096,22 +1089,9 @@ void HttpClientResponse::parse(std::string_view view, bool closed) {
     clear_content_on_error();
 }
 
-void HttpClientResponse::store(LuaStack &LS, LuaSlot tab) const {
+void HttpParser::parse_request(std::string_view view, bool closed) {
-    store_parsed(LS, tab);
-}
-
-void HttpClientResponse::store_fail(LuaStack &LS, LuaSlot tab, int status_code, std::string_view error) {
-    HttpClientResponse response;
-    response.fail(status_code, error);
-    response.store(LS, tab);
-}
-
-HttpServerRequest::HttpServerRequest() {
-    is_request_ = true;
-}
-
-void HttpServerRequest::parse(std::string_view view, bool closed) {
     std::string_view original_view = view;
+    is_request_ = true;
 
     // Parse the request line.
     if (!parse_request_line(view, closed)) {
@@ -1131,7 +1111,7 @@ void HttpServerRequest::parse(std::string_view view, bool closed) {
     }
 
     // Calculate the comm length.
-    set_comm_length(original_view.size() - view.size());
+    comm_length_ = original_view.size() - view.size();
 
     // Always ignore location.
     location_.clear();
@@ -1150,15 +1130,10 @@ void HttpServerRequest::parse(std::string_view view, bool closed) {
     clear_content_on_error();
 }
 
-void HttpServerRequest::store(LuaStack &LS, LuaSlot tab) const {
+void HttpParser::store_fail(LuaStack &LS, LuaSlot tab, int status_code, std::string_view error) {
-    store_parsed(LS, tab);
+    HttpParser parser;
-}
+    parser.fail(status_code, error);
-
+    parser.store(LS, tab);
-eng::string HttpServerRequest::DebugString() const {
-    eng::ostringstream oss;
-    oss << "HttpServerRequest:" << std::endl;
-    parser_generate_debug_string(oss);
-    return oss.str();
 }
 
 LuaDefine(http_fixurl, "url", "validate URL and repair minor flaws in the URL syntax") {
@@ -1264,9 +1239,9 @@ LuaDefine(http_clientresponse, "response",
     LuaArg text;
     LuaRet tab;
     LuaStack LS(L, text, tab);
-    HttpClientResponse resp;
+    HttpParser parser;
-    resp.parse(LS.ckstring(text), true);
+    parser.parse_response(LS.ckstring(text), true);
-    resp.store(LS, tab);
+    parser.store(LS, tab);
     return LS.result();
 }
 
@@ -1281,7 +1256,6 @@ LuaDefine(http_serverrequest, "request",
     "|  params    - a table of url-decoded URL parameters"
     "|  content   - the content, as a string (POST only)"
     "|  mimetype  - the mime type of the content (POST only)"
-    "|  location  - for HTTP redirects, the target url."
     "|"
     "|If the mimetype is a text mimetype, then the content"
     "|is automatically converted to utf-8."
@@ -1320,9 +1294,9 @@ LuaDefine(http_serverrequest, "request",
     LuaArg text;
     LuaRet tab;
     LuaStack LS(L, text, tab);
-    HttpServerRequest req;
+    HttpParser parser;
-    req.parse(LS.ckstring(text), true);
+    parser.parse_request(LS.ckstring(text), true);
-    req.store(LS, tab);
+    parser.store(LS, tab);
     return LS.result();
 }
 

luprex/core/cpp/http.hpp

@@ -147,8 +147,12 @@ public:
 //
 class HttpParser : public eng::nevernew {
 protected:
-    // True if this is for parsing a request.
+    // The request ID is not used by the parser, but
-    // This is only used for generating error messages.
+    // you can store a request ID in there for convenience.
+    int64_t request_id_;
+
+    // True if this parser parsed a request.  If false,
+    // then it parsed a response.
     bool is_request_;
 
     // The status code, a 3-digit number.
@@ -260,28 +264,27 @@ protected:
     //
     bool parse_content(std::string_view &view, bool closed);
 
-    // Get or set the communication length.
+public:
-    //
-    int comm_length() const { return comm_length_; }
-    void set_comm_length(int len) { comm_length_ = len; }
-
     // Construct a blank parser.
     //
     HttpParser();
 
     // Store the parsed fields into a lua table.
     //
-    void store_parsed(LuaStack &LS, LuaSlot tab) const;
+    void store(LuaStack &LS, LuaSlot tab) const;
 
-    // Emit all the parser fields as a debug string.
-    //
-    void parser_generate_debug_string(std::ostream &oss) const;
-
-public:
     // The parser will not try to parse content longer than this.
     //
     const int64_t MAX_CONTENT_LENGTH = 1000000;
 
+    // Parse a request or a response.
+    //
+    // You can only parse once.  If you need to parse again,
+    // construct a new parser.
+    //
+    void parse_request(std::string_view view, bool closed);
+    void parse_response(std::string_view view, bool closed);
+
     // Store a status code and an error message, and clear the content.
     // This is generally used when the client detects an error,
     // such as a DNS lookup fail, a connection failed, an SSL negotiation
@@ -289,101 +292,31 @@ public:
     //
     void fail(int status, std::string_view error);
 
-    // Return true if the communication was complete.
+    // Get or set the request ID.
-    //
-    bool complete() const { return status_ != 0; }
-};
-
-class HttpClientResponse : public HttpParser {
-private:
-    // Most of the data is stored in the HttpParser.
-    // That includes the status code, error, headers, and content.
-    
-    // The request ID.
-    int64_t request_id_;
-
-    // The length in bytes of the entire communication.
-    // If the response is complete, but the comm_length_
-    // is zero, it means we couldn't find the end of 
-    // the request.
-    //
-    int comm_length_;
-
-public:
-    // Construct a blank response.
-    HttpClientResponse();
-
-    // Parse the HTTP response.  The closed flag is to be set to true if the
-    // remote has closed the connection.
-    //
-    // If the request is incomplete, generates a status code of zero. In that
-    // case, loading more data from the server might improve the situation.
-    //
-    // If successful, indicates how much of the text was consumed by
-    // setting comm_length_.
-    //
-    void parse(std::string_view text, bool closed);
-
-    // Get or Set the request ID.
-    // This class does nothing with the request ID, it just stores it.
     //
     int64_t request_id() const { return request_id_; }
     void set_request_id(int64_t v) { request_id_ = v; }
 
-    // Convert the HTTP response to a lua table.
+    // Return true if the communication was complete.
     //
-    void store(LuaStack &LS, LuaSlot tab) const;
+    bool complete() const { return status_ != 0; }
 
-    // Convert to a debug string.
+    // Return a debug string.
     //
-    eng::string DebugString() const;
+    eng::string debug_string() const;
 
-    // Synthesize an error response and store it in lua.
+    // Synthesize an error and store it in lua.
     //
     static void store_fail(LuaStack &LS, LuaSlot tab, int status, std::string_view error);
 };
 
-class HttpServerRequest : public HttpParser {
-private:
-    // The length in bytes of the entire communication.
-    // If the request is complete, but the comm_length_
-    // is zero, it means we couldn't find the end of 
-    // the request.
-    //
-    int comm_length_;
-
-public:
-    // Construct a blank request.
-    //
-    HttpServerRequest();
-
-    // Parse the HTTP request.  The closed flag is to be set to true if the
-    // remote has closed the connection.
-    //
-    // If the request is incomplete, generates a status code of zero. In that
-    // case, loading more data from the server might improve the situation.
-    //
-    // If successful, indicates how much of the text was consumed by
-    // setting comm_length.
-    //
-    void parse(std::string_view text, bool closed);
-    
-    // Convert the HTTP request to a lua table.
-    //
-    void store(LuaStack &LS, LuaSlot tab) const;
-
-    // Convert to a debug string.
-    //
-    eng::string DebugString() const;
-};
-
 class HttpClientRequestMap : public eng::map<int64_t, HttpClientRequest> {
 public:
     void serialize(StreamBuffer *sb) const;
     void deserialize(StreamBuffer *sb);
 };
 
-using HttpClientResponseVec = eng::vector<HttpClientResponse>;
+using HttpParserVec = eng::vector<HttpParser>;
 
 // This class associates an HTTP request to an actual communication
 // channel that is executing that request.

luprex/core/cpp/lpxserver.cpp

@@ -215,17 +215,17 @@ public:
         }
 
         // Maintain existing outgoing HTTP client requests.
-        HttpClientResponseVec http_responses;
+        HttpParserVec http_responses;
         for (auto &pair : http_client_channels_) {
             HttpClientChannel &htchan = pair.second;
             Channel &channel = *htchan.channel_;
             if (channel.closed() || (channel.in()->fill() > htchan.parsed_bytes_)) {
-                HttpClientResponse response;
+                HttpParser response;
                 if (!channel.error().empty()) {
                     response.fail(503, util::ss("Service Unavailable: ", channel.error()));
                 } else {
                     htchan.parsed_bytes_ = channel.in()->fill();
-                    response.parse(channel.in()->view(), channel.closed());
+                    response.parse_response(channel.in()->view(), channel.closed());
                 }
                 if (response.complete()) {
                     response.set_request_id(pair.first);
@@ -233,7 +233,7 @@ public:
                 }
             }
         }
-        for (const HttpClientResponse &response : http_responses) {
+        for (const HttpParser &response : http_responses) {
             http_client_channels_.erase(response.request_id());
         }
         master_->http_responses(http_responses);

luprex/core/cpp/world-accessor.cpp

@@ -670,7 +670,7 @@ LuaDefine(http_get, "request",
     req.set_defaults();
     eng::string error = req.check();
     if (!error.empty()) {
-        HttpClientResponse::store_fail(LS, response, 400, util::ss("bad request: ", error));
+        HttpParser::store_fail(LS, response, 400, util::ss("bad request: ", error));
         return LS.result();
     }
 

luprex/core/cpp/world-core.cpp

@@ -393,7 +393,7 @@ void World::update_source(const util::LuaSourceVec &source) {
     assert(stack_is_clear());
 }
 
-void World::http_response(const HttpClientResponse &response) {
+void World::http_response(const HttpParser &response) {
     // Find the request.
     auto iter = http_requests_.find(response.request_id());
     if (iter == http_requests_.end()) {
@@ -443,14 +443,14 @@ void World::http_response(const HttpClientResponse &response) {
     run_scheduled_threads();
 }
 
-void World::http_responses(const HttpClientResponseVec &responses) {
+void World::http_responses(const HttpParserVec &responses) {
-    for (const HttpClientResponse &response : responses) {
+    for (const HttpParser &response : responses) {
         http_response(response);
     }
 }
 
 void World::abort_all_http_requests(int status_code, std::string_view error) {
-    HttpClientResponse abortresponse;
+    HttpParser abortresponse;
     abortresponse.fail(status_code, error);
     while (!http_requests_.empty()) {
         abortresponse.set_request_id(http_requests_.begin()->first);

luprex/core/cpp/world.hpp

@@ -206,8 +206,8 @@ public:
     void update_source(const util::LuaSourceVec &source);
     
     // Supply an HTTP response to an outstanding HTTP request.
-    void http_response(const HttpClientResponse &response);
+    void http_response(const HttpParser &response);
-    void http_responses(const HttpClientResponseVec &responses);
+    void http_responses(const HttpParserVec &responses);
     
     // Abort all HTTP requests.  This is typically used after
     // reloading a world from a save-game.  The http requests that