drogonframework / drogon

Drogon: A C++14/17/20 based HTTP web application framework running on Linux/macOS/Unix/Windows
MIT License
11.62k stars 1.12k forks source link

No way to differentiate between HEAD and GET requests #1728

Closed Greisby closed 1 year ago

Greisby commented 1 year ago

Description of the bug In the controller, there is no way to add a method for drogon::HttpMethod::Head, neither to know that a drogon::Get method was called from a HEAD request.

To Reproduce Example:

class DrogonController : public drogon::HttpController<DrogonController, false> {
    METHOD_LIST_BEGIN
    ADD_METHOD_TO(DrogonController::doGet, "/", drogon::Get);
    ADD_METHOD_TO(DrogonController::doHead, "/", drogon::Head);
    METHOD_LIST_END
    void doGet(const drogon::HttpRequestPtr& request, std::function<void(const drogon::HttpResponsePtr&)>&&);
    void doHead(const drogon::HttpRequestPtr& request, std::function<void(const drogon::HttpResponsePtr&)>&&);
}

The doGet() method is systematically called, whether the request method is GET or HEAD, and the request->method() is also always drogon::Get It is therefore impossible to know if a GET or HEAD request was done.

Expected behavior: Be able do differentiate between them, to avoid the overhead of generating a body that is then dropped.

Proposed solution: Either:

Desktop:

Greisby commented 1 year ago

Well, I found in HttpServer.cc the following:

        bool isHeadMethod = (req->method() == Head);
        if (isHeadMethod)
        {
            req->setMethod(Get);
        }

and then a lots of references to isHeadMethod, probably to drop the body. I have no idea of the implications of removing this setMethod(Get). Perhaps that the safest solution would be to add an extra property in HttpRequestImpl.h:

class HttpRequestImpl : public HttpRequest
{
    bool isHead() const { return head_; }
    void setMethod(const HttpMethod method) override
    {
        method_ = method;
        head_ |= (method == Head);
    }
private:
    bool head_{ false };
Greisby commented 1 year ago

I made a proposal in PR #1743