Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Url parser problem #347

Closed
thumb2 opened this issue Feb 18, 2022 · 5 comments · Fixed by #349
Closed

Url parser problem #347

thumb2 opened this issue Feb 18, 2022 · 5 comments · Fixed by #349
Labels
bug Something isn't working

Comments

@thumb2
Copy link

thumb2 commented Feb 18, 2022

(2022-02-18 08:12:02) [INFO ] Request: 10.5.5.2:51553 0x5650de036360 HTTP/1.1 GET /api/load-project
(2022-02-18 08:12:02) [DEBUG ] - MESSAGE: hello
(2022-02-18 08:12:02) [DEBUG ] Matched rule '/api/load-project' 1 / 10
(2022-02-18 08:12:02) [INFO ] Loading project test1
(2022-02-18 08:12:02) [INFO ] {"projectName":"test1","imageName":"20220127162725.png","worldOrigin":{"y":0,"x":0},"ratioWorldImage":1,"locators":[]}
(2022-02-18 08:12:02) [INFO ] Response: 0x5650de036360 /api/load-project?projectName=test1 200 0
(2022-02-18 08:12:02) [DEBUG ] 0x5650de036360 timer cancelled: 0x7ffa95777ac0 4
(2022-02-18 08:12:02) [DEBUG ] task_timer cancelled: 0x7ffa95777ac0 4
(2022-02-18 08:12:02) [DEBUG ] task_timer scheduled: 0x7ffa95777ac0 5
(2022-02-18 08:12:02) [DEBUG ] 0x5650de036360 timer added: 0x7ffa95777ac0 5
(2022-02-18 08:12:02) [DEBUG ] 0x5650de036360 timer cancelled: 0x7ffa95777ac0 5
(2022-02-18 08:12:02) [DEBUG ] task_timer cancelled: 0x7ffa95777ac0 5
(2022-02-18 08:12:02) [INFO ] Request: 10.5.5.2:51553 0x5650de036360 HTTP/1.1 GET /img/test1/202201
(2022-02-18 08:12:02) [DEBUG ] - MESSAGE: hello
(2022-02-18 08:12:02) [DEBUG ] Matched rule '/img/' 1 / 2
(2022-02-18 08:12:02) [INFO ] req.url_params: [ ]
(2022-02-18 08:12:02) [INFO ] req.url: /img/test1/202201
(2022-02-18 08:12:02) [INFO ] req.raw_url: /img/test1/20220127162725.png

(2022-02-18 08:12:02) [INFO ] ../project/test1/202201
(2022-02-18 08:12:02) [INFO ] -1
(2022-02-18 08:12:02) [INFO ] ../project/test1/202201
(2022-02-18 08:12:02) [INFO ] Response: 0x5650de036360 /img/test1/20220127162725.png 404 0

GET /api/load-project?projectName=test1
followed by:
GET /img/test1/20220127162725.png
Raw_url of the second GET is "/img/test1/20220127162725.png", however, url is parsed as "/img/test1/202201". Seems that the "qs_point" in http_parser is not reset to 0 before the second GET request.

@The-EDev
Copy link
Member

I haven't looked deeply into this (I've been away all day), but one line that caught my attention was (2022-02-18 08:12:02) [INFO ] ../project/test1/202201. Are you trying to navigate out of a directory by any chance?

@The-EDev The-EDev added the further information required existing information is lacking, or feedback is required label Feb 21, 2022
@thumb2
Copy link
Author

thumb2 commented Feb 21, 2022

I haven't looked deeply into this (I've been away all day), but one line that caught my attention was (2022-02-18 08:12:02) [INFO ] ../project/test1/202201. Are you trying to navigate out of a directory by any chance?

Yes, this is a tool used internally only, and "set_static_file_info_unsafe" is good enough for me.

The problem is that the second req.url is parsed using the parameter separator position of the pervious request.

/api/load-project?projectName=test1 => previous req.raw_url
/api/load-project                   => previous req.url
/img/test1/202201                   => req.url
/img/test1/20220127162725.png       => req.raw_url

The req.raw_url is correct (/img/test1/20220127162725.png), however, the req.url is wrong.

As a workaround, the following code works (some codes generating the previous log are commented) :

 286 │     CROW_ROUTE(app, "/img/<path>")(
 287 │         [](const crow::request& req, crow::response& res, std::string path)
 288 │         {
 289 │             std::string filename;
 290// CROW_LOG_INFO << "req.url_params: " << req.url_params;
 291// CROW_LOG_INFO << "req.url: " << req.url;
 292// CROW_LOG_INFO << "req.raw_url: " << req.raw_url;
 293 │             path = req.raw_url.substr(5);   // Replace "/img/" by "../project/"
 294 │             filename = "../project/" + path;
 295// CROW_LOG_INFO << filename;
 296// struct stat statbuf;
 297// auto statResult = stat(filename.c_str(), &statbuf);
 298// CROW_LOG_INFO << statResult;
 299// CROW_LOG_INFO << filename.c_str();
 300 │             res.set_static_file_info_unsafe(filename);
 301 │             res.end();
 302 │         });

@The-EDev The-EDev added bug Something isn't working and removed further information required existing information is lacking, or feedback is required labels Feb 21, 2022
@The-EDev
Copy link
Member

The-EDev commented Feb 21, 2022

@thumb2 Thank you for the information!

I should mention I attempted to recreate your issue (using only the image name and one route) and failed to do so (the file was returned correct).

The code and results you provided just now will help in investigating this issue greatly.

One peculiar thing is that req.url has the same length in both cases, so it's most likely the new parser's qs_point not being cleared after a request.

Once again thank you very much for reporting this bug, I'll make sure it's fixed as soon as possible.

@thumb2
Copy link
Author

thumb2 commented Feb 22, 2022

Thanks for the quick fix. Your PR works:

 559 │ (2022-02-22 02:54:35) [INFO    ] Request: 10.5.5.2:65085 0x7fc018000e70 HTTP/1.1 GET /img/test1/20220127162725.png
 560 │ (2022-02-22 02:54:35) [DEBUG   ]  - MESSAGE: hello
 561 │ (2022-02-22 02:54:35) [DEBUG   ] Matched rule '/img/<path>' 1 / 2
 562 │ (2022-02-22 02:54:35) [INFO    ] req.url_params: [  ]
 563 │ (2022-02-22 02:54:35) [INFO    ] req.url: /img/test1/20220127162725.png
 564 │ (2022-02-22 02:54:35) [INFO    ] req.raw_url: /img/test1/20220127162725.png
 565 │ (2022-02-22 02:54:35) [INFO    ] path: test1/20220127162725.png

BTW, I'm a newbie of HTTP server developing, which Crow branch should I use for now?

@The-EDev
Copy link
Member

while master is a bit unstable at the moment, we're so close to a release that I would still recommend it. We've added a lot of features since v0.3 that using master is easier and more convenient, even if it has a bug or 2 (which will be fixed soon I promise).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants