drogonframework / drogon

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

fix a bug in plugin Redirector. #2198

Closed tanglong3bf closed 1 week ago

tanglong3bf commented 3 weeks ago
#include <drogon/drogon.h>
#include <drogon/plugins/Redirector.h>

int main()
{
    drogon::app().addListener("0.0.0.0", 8000);
    // {"plugins": [{"name": "drogon::plugin::Redirector"}]}
    drogon::app().loadConfigFile("../config.json");
    drogon::app().registerBeginningAdvice([] {
        drogon::plugin::Redirector *redirector =
            drogon::app().getPlugin<drogon::plugin::Redirector>();

        redirector->registerRedirectHandler(
            [](const drogon::HttpRequestPtr &req,
               std::string &protocol,
               std::string &host,
               bool &pathChanged) -> bool {
                if (req->path() == "/")
                {
                    protocol = "https://";
                    host = "github.com";
                    req->setPath("/drogonframework/drogon");
                    pathChanged = true;
                }
                return true;
            });

        redirector->registerPathRewriteHandler(
            [](const drogon::HttpRequestPtr &req) -> bool {
                if (!req->path().starts_with("/api") && req->path() != "/")
                {
                    req->setPath("/");
                    return true;
                }
                return false;
            });
    });
    drogon::app().run();
    return 0;
}

For the above code, the expected goal is: access localhost:8000, redirect to https://github.com/drogonframework/drogon. The old version of the Redirector plugin redirects to the github.com homepage, because the loop of pathRewriteHandlers_ modifies the path. This is not as expected.

Mis1eader-dev commented 3 weeks ago

Will try to give it a review once I'm back home (╯°□°)╯︵ ┻━┻

P.S. the emoticon is just for fun

Mis1eader-dev commented 2 weeks ago

The reason this happens is

[](const drogon::HttpRequestPtr &req) -> bool {
                if (!req->path().starts_with("/api") && req->path() != "/")
                {
                    req->setPath("/");
                    return true;
                }
                return false;
            }

Triggers right after the redirect logic populates protocol, host, and new path to GitHub. image

Hence your check ends up overwriting the path to "/".

I'm unsure of the consequences of changing the orders, it may break other plugins in the process.

I'll do further tests and confirm. Or we could just add another handler to the pipeline and call it a day lol