Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/test.yml-template
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Test

on:
pull_request:
branches: [ master ]

jobs:
build:

runs-on: ubuntu-latest

strategy:
matrix:
node-version: [20.x]

steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node-version }}
- run: npm install
- run: npm test
9 changes: 5 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"devDependencies": {
"@faker-js/faker": "^8.4.1",
"@mate-academy/eslint-config": "latest",
"@mate-academy/scripts": "^1.8.6",
"@mate-academy/scripts": "^2.1.3",
"axios": "^1.7.2",
"eslint": "^8.57.0",
"eslint-plugin-jest": "^28.6.0",
Expand Down
2 changes: 1 addition & 1 deletion src/app.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable no-console */
'use strict';

const { createServer } = require('./createServer');
const { createServer } = require('./createServer.js');

createServer().listen(5701, () => {
console.log(`Server is running on http://localhost:${5701} 🚀`);
Expand Down
73 changes: 68 additions & 5 deletions src/createServer.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,73 @@
/* eslint-disable no-console */
'use strict';

const http = require('http');
const path = require('path');
const fsp = require('fs/promises');

function createServer() {
/* Write your code here */
// Return instance of http.Server class
const server = http.createServer(async (req, res) => {
const MIME_TYPES = {
'.html': 'text/html',
'.css': 'text/css',
'.js': 'application/javascript',
'.json': 'application/json',
'.png': 'image/png',
'.jpg': 'image/jpeg',
'.svg': 'image/svg+xml',
};

if (req.url.includes('//')) {
res.setHeader('Content-Type', 'text/plain');
res.statusCode = 404;
res.end('Not Found');

return;
}

const pathname = req.url.slice(1);

const [action, ...rest] = pathname.split('/');
const fileName = rest.join('/');

if (action !== 'file') {
res.setHeader('Content-Type', 'text/plain');
res.statusCode = 400;

res.end('To load file, you need use "/file/fileName"');

return;
}

if (!fileName) {
res.setHeader('Content-Type', 'text/plain');
res.statusCode = 200;
res.end('To load file, you need use "/file/fileName"');

return;
Comment on lines +42 to +47
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conditional check is currently unreachable. The logic on line 22 (|| 'index.html') ensures that fileName will always be a non-empty string, so this if block will never execute. You can safely remove it.

}
Comment on lines +42 to +48
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the requirements, requests to /file/ and /file should return public/index.html. This logic incorrectly returns a message instead. You should modify this block to set the fileName to index.html when it's empty, and then allow the code to proceed to the file reading part.

Comment on lines +42 to +48
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The requirements state that requests to /file and /file/ should serve public/index.html. This logic currently returns a hint message instead. Consider what fileName should be in this case to allow the existing file-reading logic to handle it correctly.


const realPath = path.join(__dirname, '..', 'public', fileName);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation is vulnerable to path traversal attacks. A request like /file/../../package.json could allow access to files outside of the public directory, which violates the requirements. You need to validate that the resolved file path is still within the public folder before serving the file.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path traversal vulnerability mentioned in the previous review is still present. A malicious request like /file/../../package.json could access files outside the public directory. You must validate that the final realPath is still within the public folder before attempting to read the file. A common way to do this is to resolve the absolute paths and check if the requested file path starts with the public directory's absolute path.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation is still vulnerable to path traversal. A malicious request like /file/../../package.json can access files outside of the public directory. To fix this, you need to ensure the final resolved path is actually inside the public folder before serving the file. You can use path.resolve() to get the absolute path and then verify it.


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

const ext = path.extname(fileName);
const contentType = MIME_TYPES[ext] || 'text/plain';

res.setHeader('Content-Type', contentType);

res.statusCode = 200;
res.end(file);
} catch (error) {
res.setHeader('Content-Type', 'text/plain');
res.statusCode = 404;

res.end('Not Found');
}
});

return server;
}

module.exports = {
createServer,
};
module.exports = { createServer };
Loading