ipkn / crow

Crow is very fast and easy to use C++ micro web framework (inspired by Python Flask)
BSD 3-Clause "New" or "Revised" License
7.48k stars 891 forks source link

Route selection error for sub resources #215

Open cparich opened 7 years ago

cparich commented 7 years ago

The following code does not pick the correct route on GET localhost:18080/test/path

#include <crow.h>

int main() {
    crow::SimpleApp app;

    CROW_ROUTE(app, "/test/<string>")
            .methods("GET"_method)
            ([ = ](crow::request req, std::string str){
        return crow::response(200, str);
    });

    CROW_ROUTE(app, "/test/path")
            .methods("GET"_method)
            ([ = ](crow::request req){
        return crow::response(400);
    });

    app.port(18080).multithreaded().run();
}

Expected result is Error 400

Actual result is 200 OK / "path" in body

compiled with G++ v5.4.0 via g++ -std=c++14 -I../crow main.cpp -lboost_system -lpthread -o crow_error

Shravan40 commented 7 years ago

200 OK is correct result in this case.

CROW_ROUTE(app, "/test/<string>")
            .methods("GET"_method)
            ([ = ](crow::request req, std::string str){
        return crow::response(200, str);
    });

This is correct path for GET localhost:18080/test/path. Because whenever you write app route path with /<datatype> and matching datatype has been passed in URL path it will match.

cparich commented 7 years ago

This conflicts with most other implementations of REST, including Jersey and WebAPI2.

It also makes handling routes such as /employees, /employees/<id>, /employees/time, and /employees/<id>/time incoherent inside the application where "id" is not an integer

cparich commented 7 years ago

To add to this, the error seems to be inside the Trie object's find() method. All routes register successfully, which is important since if there were duplicate sub-routes, per @Shravan40's suggestion that /path/<string> and /path/subpath are the same, then Crow would throw a runtime exception indicating as much.

cparich commented 7 years ago

After a little more testing, it looks like this issue is path binding order-dependent, so for instance the following code works as expected with

GET localhost:18080/test/path returning 400 Error and GET localhost:18080/test/path1 returning 200 OK / "path1" in body

#include <crow.h>

int main() {
    crow::SimpleApp app;

    CROW_ROUTE(app, "/test/path")
            .methods("GET"_method)
            ([ = ](crow::request req){
        return crow::response(400);
    });

    CROW_ROUTE(app, "/test/<string>")
            .methods("GET"_method)
            ([ = ](crow::request req, std::string str){
        return crow::response(200, str);
    });

    app.port(18080).multithreaded().run();
}

notice the reversal of the route definitions. Definitely the cause of why I had initial trouble reproducing this problem.

I've added a workaround in my code to define all non-parameterized paths first.