Solution#375
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has two critical issues that violate the task requirements. First, the path traversal check returns status code 404 for ../ sequences, but the requirements explicitly state it should return 400 for paths containing ../. Change res.statusCode = 404 to res.statusCode = 400 in the path traversal block (around lines 58-63). Second, the duplicated slashes validation (//) is commented out on lines 29-34, causing /file//styles//main.css to incorrectly return 200 instead of the required 404 status. Remove the comment delimiters to enable this check. Both of these are explicit requirements from the task description that would cause test failures.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| // защита от path traversal | ||
| if (!fullPath.startsWith(publicDir + path.sep) && fullPath !== publicDir) { |
There was a problem hiding this comment.
The path traversal protection returns 404, but according to the task requirements, paths containing ../ should return 400 status. Change res.statusCode = 404 to res.statusCode = 400.
|
|
||
| // duplicated slashes | ||
| /* if (pathName.includes('//')) { | ||
| res.statusCode = 404; | ||
| res.setHeader('Content-Type', 'text/plain'); | ||
|
|
||
| return res.end(); |
There was a problem hiding this comment.
The duplicated slashes check is commented out. The task requires returning 404 for paths containing // (like /file//styles//main.css). Remove the comment delimiters to enable this validation.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation now correctly addresses the two critical issues from the previous review: path traversal (../) returns 400 status, and duplicated slashes (//) validation is enabled returning 404 status. All core requirements are met: file serving from the public folder works correctly, proper Content-Type headers are set, and edge cases are handled appropriately. One minor inconsistency remains: /file returns 200 with hint, while /anything returns 400 with hint. While this isn't a blocker (tests may not specifically check for this), consider aligning both to return 200 since hints are informational rather than errors. Overall, your solution is approved. The critical blockers have been resolved, and the server now handles all specified request scenarios correctly.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.