Solution by alyonashunevych · Pull Request #346 · mate-academy/node_static-files-server

alyonashunevych · 2026-02-23T21:56:52Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there is a critical path traversal vulnerability that breaks the core requirement of serving files only from the public directory ([CHECKLIST ITEM #2], [CHECKLIST ITEM #3], [CHECKLIST ITEM #9], [CHECKLIST ITEM #10]). Right now, the extracted requestedPath can still contain .. segments (e.g. /file/../secret.txt), and you directly join this into realPath, which allows access to files outside public. To fix this, normalize and resolve the path using something like const realPath = path.resolve(publicDir, requestedPath) and then verify that realPath starts with the resolved publicDir path; if it doesn’t, immediately return a 404. Once you add that, you can remove the url.pathname.includes('//') check, since the resolved-path check makes it unnecessary and more robust.

On the positive side, you correctly route /file and /file/ to index.html and return 404 when file reads fail, which matches several functional requirements. As a follow-up improvement (not blocking), avoid forcing 'utf-8' when reading files and set Content-Type based on the file extension so that binary files and HTML/CSS are served correctly. With the path traversal protection in place and the routing kept as-is, your server will meet the key security and correctness requirements for this task.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-02-23T22:00:08Z

src/createServer.js

+      return;
+    }
+
+    if (url.pathname.includes('//')) {


Checking url.pathname.includes('//') isn't a reliable defense against path traversal or attempts to escape public. Use path normalization and a resolved-path check instead.

mateacademy-ai-mentor · 2026-02-23T22:00:08Z

src/createServer.js

+    const requestedPath =
+      url.pathname.replace(/^\/file\/?/, '') || 'index.html';


The requestedPath extraction doesn't strip or reject .. segments — url.pathname.replace(...) can still produce values like ../secret.txt. Normalize or validate this segment before using it.

mateacademy-ai-mentor · 2026-02-23T22:00:08Z

src/createServer.js

+    const requestedPath =
+      url.pathname.replace(/^\/file\/?/, '') || 'index.html';
+
+    const realPath = path.join(__dirname, '..', 'public', requestedPath);


Joining the (unvalidated) requestedPath into realPath allows escaping public. After computing realPath with path.resolve, verify it is inside the resolved public directory (e.g. compare prefixes) and return 404 if not. This addresses checklist items #3 and #10.

mateacademy-ai-mentor · 2026-02-23T22:00:08Z

src/createServer.js

+    const realPath = path.join(__dirname, '..', 'public', requestedPath);
+
+    try {
+      const file = await fsp.readFile(realPath, 'utf-8');


Reading files with 'utf-8' will corrupt binary files (images, etc.). If you may serve binary files, read without an encoding (Buffer) and set Content-Type appropriately based on extension.

mateacademy-ai-mentor · 2026-02-23T22:00:08Z

src/createServer.js

+    const url = new URL(req.url || '', `http://${req.headers.host}`);
+    const isValidURL = /^\/file/.test(url.pathname);
+
+    res.setHeader('Content-Type', 'text/plain');


The code sets Content-Type to text/plain for every response. For better behavior set MIME types according to the file extension (e.g. text/html for .html) or at minimum set text/html for index.html responses.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there is still a critical security and correctness issue that violates the core requirement of serving files only from the public directory. In src/createServer.js, the current path.resolve('public', requestedPath) without a containment check allows path traversal like /file/../secret.txt, which can escape the public folder; you must instead resolve a publicDir once, then resolve realPath against it, and verify via path.relative(publicDir, realPath) that the result does not start with .. and is not absolute before serving the file. The route check using /^\/file/ is also too permissive and incorrectly treats paths like /fileX as valid; tighten this to something like ^/file(?:/|$) or explicitly check url.pathname === '/file' || url.pathname.startsWith('/file/') so that only the intended paths are mapped and other paths get the hint message.

You also need to fix content handling so the server behaves correctly for different file types: after computing realPath, determine the extension and set an appropriate Content-Type (e.g. .html -> text/html; charset=utf-8, .css -> text/css; charset=utf-8, else application/octet-stream) right before sending the response, instead of always using text/plain. When reading the file, avoid forcing UTF-8 by calling fsp.readFile(realPath) without an encoding and then sending the resulting Buffer with res.end(file) so binary files (images, etc.) are not corrupted. If you want to improve robustness further, you can distinguish ENOENT (404) from other errors (e.g. permission issues → 500), but the main blockers are the path traversal, the overly broad /file matching, and the incorrect content-type/encoding behavior. Once these are addressed, your overall structure (mapping /file, /file/, and /file/... into public and returning 404 for missing files) will fully meet the task requirements—you're very close, just need these corrections.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-02-23T22:14:05Z

src/createServer.js

+const createServer = () => {
+  return http.createServer(async (req, res) => {
+    const url = new URL(req.url || '', `http://${req.headers.host}`);
+    const isValidURL = /^\/file/.test(url.pathname);


The regex /^\/file/ matches strings like /fileX, which is not intended. The requirement expects requests that are exactly /file or start with /file/. Use a stricter check such as ^/file(?:/|$) or explicitly url.pathname === '/file' || url.pathname.startsWith('/file/') so non-/file paths get the hint message as required.

mateacademy-ai-mentor · 2026-02-23T22:14:05Z

src/createServer.js

+    const url = new URL(req.url || '', `http://${req.headers.host}`);
+    const isValidURL = /^\/file/.test(url.pathname);
+
+    res.setHeader('Content-Type', 'text/plain');


Setting Content-Type to text/plain for all responses prevents correct rendering of HTML/CSS and breaks binary responses. Determine the file extension after resolving realPath and set an appropriate Content-Type (e.g. .html → text/html; charset=utf-8, .css → text/css; charset=utf-8, otherwise application/octet-stream). Set the header right before sending the file.

mateacademy-ai-mentor · 2026-02-23T22:14:05Z

src/createServer.js

+    const requestedPath =
+      url.pathname.replace(/^\/file\/?/, '') || 'index.html';
+
+    const realPath = path.resolve('public', requestedPath);


Using path.resolve('public', requestedPath) without verifying the result allows path traversal (e.g. /file/../secret.txt). Fix by computing const publicDir = path.resolve('public'), then const realPath = path.resolve(publicDir, requestedPath), and verify realPath is inside publicDir (e.g. path.relative(publicDir, realPath) should not start with .. and should not be absolute). If it's outside, return 404 immediately. This is required so the server returns only files from public.

mateacademy-ai-mentor · 2026-02-23T22:14:05Z

src/createServer.js

+    const realPath = path.resolve('public', requestedPath);
+
+    try {
+      const file = await fsp.readFile(realPath, 'utf-8');


Calling fsp.readFile(realPath, 'utf-8') forces UTF-8 text encoding and breaks binary files. Read files as raw buffers (await fsp.readFile(realPath)) and then res.end(file) (a Buffer). Combined with proper Content-Type, this lets you serve images and other binary assets correctly.

mateacademy-ai-mentor · 2026-02-23T22:14:05Z

src/createServer.js

+    } catch (error) {
+      res.statusCode = 404;
+      res.end('Not Found');


When readFile throws, you always return 404. It's correct to return 404 for ENOENT (missing files), but consider distinguishing other errors (e.g. permission errors → 500) for clearer responses and easier debugging. Not strictly required, but helpful.

mateacademy-ai-mentor

Review Summary

I am approving this submission as an educational override to help you move forward, but there are a few important issues to address before the server is fully functional. You've done a great job adding the path traversal check and correcting the route matching logic!

However, there are three key issues to resolve in src/createServer.js:

Missing return: After sending a 404 response for an invalid path, you must add a return; to stop the function and prevent a server crash.
Hardcoded Content-Type: The header is always text/plain. This should be set dynamically based on the file's extension (e.g., text/html for .html) so browsers can render content correctly.
Incorrect File Encoding: Reading files with 'utf-8' encoding will corrupt binary data like images. Please remove the encoding argument from fsp.readFile to let it read the file as a raw Buffer.

You're very close to a fully working and secure server. Keep up the excellent effort!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-02-24T22:16:54Z

src/createServer.js

+    const regexPattern = /^\/file(?:\/|$)/;
+    const isValidURL = regexPattern.test(url.pathname);
+
+    res.setHeader('Content-Type', 'text/plain');


The Content-Type is always set to text/plain. To correctly serve different file types, this header should be set dynamically based on the file's extension. For example, a .html file should have text/html, and a .css file should have text/css.

mateacademy-ai-mentor · 2026-02-24T22:16:54Z

src/createServer.js

+
+    if (path.relative(publicDir, realPath).slice(0, 2) === '..') {
+      res.statusCode = 404;
+      res.end('Not Found');


After sending the response here, you need to add return;. Without it, the function continues to the try...catch block below and will attempt to send a second response for the same request, causing a server error.

mateacademy-ai-mentor · 2026-02-24T22:16:54Z

src/createServer.js

+    }
+
+    try {
+      const file = await fsp.readFile(realPath, 'utf-8');


Specifying the 'utf-8' encoding here will cause problems with non-text files (like images), as it will corrupt their data. You should read the file as a raw Buffer by removing the encoding argument, which res.end() can handle correctly.

Solution

ac9e292

mateacademy-ai-mentor suggested changes Feb 23, 2026

View reviewed changes

Solution

d45b730

alyonashunevych requested a review from mateacademy-ai-mentor February 23, 2026 22:09

mateacademy-ai-mentor suggested changes Feb 23, 2026

View reviewed changes

Solution

710494b

alyonashunevych requested a review from mateacademy-ai-mentor February 24, 2026 22:15

mateacademy-ai-mentor approved these changes Feb 24, 2026

View reviewed changes

		const requestedPath =
		url.pathname.replace(/^\/file\/?/, '') \|\| 'index.html';

Conversation

alyonashunevych commented Feb 23, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants