3
\$\begingroup\$

Getting close to the library I want.

Here is me using the library I have been posting in chunks to build a functional Web Server. Any suggestions on code or interface changes. I want the code to look logical enough to read without much effort. I can read it because I have written it all but I want to understand if it is OK for others.

#include "NisseServer/NisseServer.h"
#include "NisseServer/AsyncStream.h"
#include "NisseHTTP/HTTPHandler.h"
#include "NisseHTTP/Request.h"
#include "NisseHTTP/Response.h"
#include "NisseHTTP/HeaderResponse.h"
#include <ThorsSocket/Server.h>
#include <filesystem>

namespace TAS       = ThorsAnvil::ThorsSocket;
namespace NServer   = ThorsAnvil::Nisse::Server;
namespace NHTTP     = ThorsAnvil::Nisse::HTTP;
namespace FS        = std::filesystem;

TAS::ServerInit getSSLInit(FS::path certPath, int port)
{
    TAS::CertificateInfo        certificate{FS::canonical(certPath /= "fullchain.pem"),
                                            FS::canonical(certPath /= "privkey.pem")
                                           };
    TAS::SSLctx                 ctx{TAS::SSLMethodType::Server, certificate};
    return TAS::SServerInfo{port, ctx};
}

TAS::ServerInit getNormalInit(int port)
{
    return TAS::ServerInfo{port};
}


int main(int argc, char* argv[])
{
    if (argc != 3 && argc != 4)
    {
        std::cerr << "Usage: WebServer <port> <contentDirectory> [<certificateDirectory>]\n";
        return 1;
    }
    try
    {
        int             port        = std::stoi(argv[1]);
        FS::path        contentDir  = FS::canonical(argv[2]);
        TAS::ServerInit serverInit  = (argc == 3) ? getNormalInit(port) : getSSLInit(argv[3], port);

        std::cout << "Nisse WebServer: Port: " << port << " ContentDir: >" << contentDir << "< Certificate Path: >" << (argc == 3 ? "NONE" : argv[3]) << "<\n";

        NHTTP::HTTPHandler   http;
        http.addPath("/{path}", [&](NHTTP::Request& request, NHTTP::Response& response)
        {
            NHTTP::HeaderResponse    header;

            std::error_code ec;
            FS::path        requestPath = FS::path{request.variables()["path"]}.lexically_normal();
            if (requestPath.empty() || (*requestPath.begin()) == "..")
            {
                response.setStatus(400);
                header.add("Error", "Invalid Request Path");
                response.addHeaders(header);
                return;
            }
            FS::path        filePath    = contentDir /= requestPath;
            FS::path        filePathCan = FS::canonical(filePath, ec);
            if (!ec && FS::is_directory(filePathCan)) {
                filePathCan = FS::canonical(filePathCan /= "index.html", ec);
            }
            if (ec || !FS::is_regular_file(filePathCan))
            {
                response.setStatus(404);
                header.add("Error", "No File Found At Path");
                response.addHeaders(header);
                return;
            }

            TAS::SocketStream       file{TAS::Socket{TAS::FileInfo{filePathCan.string(), TAS::FileMode::Read}, TAS::Blocking::No}};
            NServer::AsyncStream    async(file, request.getContext(), NServer::EventType::Read);

            response.addHeaders(header);
            response.body(NHTTP::Encoding::Chunked) << file.rdbuf();
        });

        NServer::NisseServer   server;
        server.listen(serverInit, http);

        NServer::PyntControl  control(server);
        server.listen(TAS::ServerInfo{port+2}, control);

        server.run();
    }
    catch (std::exception const& e)
    {
        std::cerr << "Exception: " << e.what() << "\n";
        throw;
    }
    catch (...)
    {
        std::cerr << "Exception: Unknown\n";
        throw;
    }
}

The main API here that is being used is the Request and Response objects that are part of the lambda.

Request

#ifndef THORSANVIL_NISSE_NISSEHTTP_REQUEST_H
#define THORSANVIL_NISSE_NISSEHTTP_REQUEST_H

#include "NisseHTTPConfig.h"
#include "URL.h"
#include "HeaderRequest.h"
#include "HeaderResponse.h"
#include "StreamInput.h"
#include "NisseServer/Context.h"
#include <istream>

namespace ThorsAnvil::Nisse::HTTP
{

class Request
{
    Server::Context*    context;
    std::string         messageHeader;
    Version             version;
    Method              method;
    URL                 url;
    HeaderRequest       head;
    HeaderRequest       tail;
    HeaderResponse      failResponse;
    RequestVariables    var;

    mutable StreamInput input;

    std::unique_ptr<std::streambuf> streamBuf;
    public:
        Request(std::string_view proto, std::istream& stream);
        Request(Server::Context& context, std::string_view proto, std::istream& stream);
        Server::Context&        getContext()    const   {return *context;}
        Version                 getVersion()    const   {return version;}
        Method                  getMethod()     const   {return method;}
        URL const&              getUrl()        const   {return url;}
        std::string_view        httpRawRequest()const   {return messageHeader;}

        HeaderRequest const&    headers()       const   {return head;}
        HeaderRequest const&    trailers()      const   {return tail;}
        HeaderResponse const&   failHeader()    const   {return failResponse;}
        bool                    isValidRequest()const   {return failResponse.empty();}

        RequestVariables&       variables()             {return var;}
                                                    // Trailers will return an empty HeaderRequest() if body has not been read.
                                                    // if (body().eof()) Then trailers have been read.

        std::istream&           body() const;       // Can be used to read the stream body.
                                                    // It will auto eof() when no more data is available in the body.
                                                    // Note this stream will auto decode the incoming message body based
                                                    // on the 'content-encoding'

        friend std::ostream& operator<<(std::ostream& stream, Request const& request)   {request.print(stream);return stream;}
        void print(std::ostream& stream) const;

    private:
        // Removed for clarity.
};

}

Response

#ifndef THORSANVIL_NISSE_NISSEHTTP_RESPONSE_H
#define THORSANVIL_NISSE_NISSEHTTP_RESPONSE_H

#include "NisseHTTPConfig.h"
#include "Util.h"
#include "HeaderResponse.h"
#include "HeaderPassThrough.h"
#include "StreamOutput.h"
#include <set>
#include <ostream>
#include <functional>

namespace ThorsAnvil::Nisse::HTTP
{

class Response
{
    Version         version;
    StatusCode      statusCode;
    bool            headerSent;

    std::ostream&   baseStream;
    StreamOutput    stream;

    public:
        Response(std::ostream& stream, Version version, int code = 200);
        ~Response();
        void                setStatus(int code);

        friend std::istream& operator>>(std::istream& stream, Response& response)  {response.read(stream);return stream;}
        void read(std::istream& stream);

        using Header = std::variant<std::reference_wrapper<HeaderResponse const>, std::reference_wrapper<HeaderPassThrough const>>;

        void addHeaders(Header const& headers);
        std::ostream& body(BodyEncoding encoding);
};

}

#endif
\$\endgroup\$

1 Answer 1

Reset to default
3
\$\begingroup\$

Make the interface simpler

The code in main() looks quite complicated to me. A lot of hoops need to be jumped just to serve a simple file, and the more code you have to write to make something work the higher the chance is for introducing bugs. Ideally, the library takes care of most things automatically, and the application just provides enough information for the library to do its thing.

Here is what I would like to see the application code look like:

#include "Nisse/Server.h"
#include "Nisse/HTTP.h"

#include <cstdlib>
#include <filesystem>
#include <fstream>

namespace Nisse = ThorsAnvil::Nisse;
namespace FS = std::filesystem;

static Nisse::Response handle_request(const Nisse::Request& request) {
    auto path = FS::path{request.getVariable("path")}.lexically_normal();

    if (path.empty() || *path.begin() == "..")
        return Nisse::Response::error(400, "Invalid request path");
    
    if (FS::is_directory(path))
        path /= "index.html";

    if (!FS::is_regular_file(path))
        return Nisse::Response::error(404, "No file found at path");

    std::ifstream file(path);
    return Nisse::Response::from_stream(file, Nisse::HTTP::Encoding::Chunked);
}

int main(int argc, char* argv[]) {
    if (argc != 3 && argc != 4) {
        std::cerr << "Usage: WebServer <port> <contentDirectory> [<certificateDirectory>]\n";
        return EXIT_FAILURE;
    }

    Nisse::Server server(argv[1], argc == 4 ? Nisse::HTTPS{argv[3]} : Nisse::HTTP{});
    FS::current_path(argv[2]);
    server.addPath("/{path}", handle_request);
    server.run();
}

The differences compared to your version:

  • Less intermediate types. No ServerInit, SocketStream and AsyncStream.
  • I didn't see the point in the async stuff since the request handler itself is not async.
  • More functional style, allowing for easier composition:
    • The request handler only takes a request, it returns the response.
    • Helper functions to generate response objects.
  • More RAII: Server takes all the information it needs to start listening in its constructor.
  • Simpler namespaces.
  • Server takes care of parsing the port number, HTTPS takes care of loading the certificate chain and private key and setting up the SSL context.

Some other changes that made the code simpler:

  • Removed the try/catch expressions. They didn't do anything useful; on most operating systems an unhandled exception will cause the program to print what() and abort anyway.
  • Changing the working directory to the content directory greatly simplifies path handling, and it's even a bit more secure: consider that someone could move/rename the content directory, or any part of the path leading up to it, while the server is running.
\$\endgroup\$
7
  • \$\begingroup\$ I didn't see the point in the async stuff since the request handler itself is not async. It is. It may look like sync code but underneath it yields the thread to do other work if the read or write block (on socket or file). \$\endgroup\$ Commented Oct 25, 2024 at 22:46
  • \$\begingroup\$ takes all the information it needs to start listening in its constructor. I tried that at first. But this only allows for one port to listen on. The current API allows you to listen on multiple ports simultaniously. \$\endgroup\$ Commented Oct 25, 2024 at 22:50
  • \$\begingroup\$ Simpler namespaces. Yea. Its getting a bit harry. I need to re-think some of that. \$\endgroup\$ Commented Oct 25, 2024 at 22:51
  • \$\begingroup\$ systems an unhandled exception will cause the program to print what() and abort anyway. True which is why I re-throw. The problem is that without the try/catch block it is implementation dependent if the stack is correctly unwound and all the destructors called. This way I know that all the destructors are correctly called. \$\endgroup\$ Commented Oct 25, 2024 at 22:53
  • \$\begingroup\$ I like the idea of returning the Response object. Let me kick that around. \$\endgroup\$ Commented Oct 25, 2024 at 22:57

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.