-
Notifications
You must be signed in to change notification settings - Fork 275
add a solution #184
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
base: master
Are you sure you want to change the base?
add a solution #184
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,160 @@ | ||
| 'use strict'; | ||
|
|
||
| const http = require('http'); | ||
| const path = require('path'); | ||
| const zlib = require('node:zlib'); | ||
| const { Readable } = require('stream'); | ||
|
|
||
| const extMap = { gzip: 'gzip', deflate: 'deflate', br: 'br' }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: const extMap = { gzip: 'gz', deflate: 'dfl', br: 'br' };This change is required so the response filename matches the task specification. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The extension map uses long names ( |
||
|
|
||
| function createServer() { | ||
| /* Write your code here */ | ||
| // Return instance of http.Server class | ||
| return http.createServer(async (req, res) => { | ||
| if (req.url === '/' && req.method === 'GET') { | ||
| res.statusCode = 200; | ||
| res.end('OK'); | ||
|
|
||
| return; | ||
|
Comment on lines
+12
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GET / currently returns the string |
||
| } | ||
|
|
||
| if (req.url !== '/compress') { | ||
| res.statusCode = 404; | ||
| res.end('Not Found'); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (req.method !== 'POST') { | ||
| res.statusCode = 400; | ||
| res.end('Incorrect form method'); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const ct = req.headers['content-type']; | ||
|
|
||
| if (!ct || !ct.includes('multipart/form-data')) { | ||
| res.statusCode = 400; | ||
| res.end('Expected multipart/form-data'); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const boundary = ct.split('boundary=')[1]?.replace(/(^"|"$)/g, ''); | ||
|
|
||
| if (!boundary) { | ||
| res.statusCode = 400; | ||
| res.end('Missing boundary'); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const chunks = []; | ||
|
|
||
| for await (const chunk of req) { | ||
| chunks.push(chunk); | ||
| } | ||
|
|
||
| const body = Buffer.concat(chunks); | ||
|
|
||
| const boundaryBuf = Buffer.from(`--${boundary}`); | ||
| let start = 0; | ||
| const parts = []; | ||
|
|
||
| while (true) { | ||
| const idx = body.indexOf(boundaryBuf, start); | ||
|
|
||
| if (idx === -1) { | ||
| break; | ||
| } | ||
|
|
||
| const nextIdx = body.indexOf(boundaryBuf, idx + boundaryBuf.length); | ||
| const end = nextIdx !== -1 ? nextIdx : body.length; | ||
|
|
||
| parts.push(body.slice(idx + boundaryBuf.length, end)); | ||
| start = end; | ||
| } | ||
|
|
||
| let fileBuffer = null; | ||
| let filename = null; | ||
| let type = null; | ||
|
|
||
| for (const part of parts) { | ||
| let p = part; | ||
|
|
||
| if (p.slice(0, 2).toString() === '\r\n') { | ||
| p = p.slice(2); | ||
| } | ||
|
|
||
| const delimiter = Buffer.from('\r\n\r\n'); | ||
| const idx = p.indexOf(delimiter); | ||
|
|
||
| if (idx === -1) { | ||
| continue; | ||
| } | ||
|
|
||
| const headersBuf = p.slice(0, idx); | ||
| const bodyBuf = p.slice(idx + delimiter.length); | ||
| const headersStr = headersBuf.toString('utf8').trim(); | ||
|
|
||
| if (headersStr.includes('name="compressionType"')) { | ||
| type = bodyBuf.toString('utf8').trim().toLowerCase(); | ||
|
|
||
| if (!['gzip', 'deflate', 'br'].includes(type)) { | ||
| res.statusCode = 400; | ||
| res.end('Unsupported compression type'); | ||
|
|
||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (headersStr.includes('name="file"')) { | ||
| const match = headersStr.match(/filename="(.+?)"/); | ||
|
|
||
| filename = match ? match[1] : 'file'; | ||
|
|
||
| let endIdx = bodyBuf.length; | ||
|
|
||
| if (bodyBuf.slice(-2).toString() === '\r\n') { | ||
| endIdx -= 2; | ||
| } | ||
| fileBuffer = bodyBuf.slice(0, endIdx); | ||
| } | ||
| } | ||
|
|
||
| if (!fileBuffer || !type) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Functional correctness: the code currently treats the presence of if (!fileBuffer || !fileBuffer.length || !type) {
// respond 400
}This ensures zero-length uploads are rejected with 400 as required. |
||
| res.statusCode = 400; | ||
| res.end('Missing file or compressionType'); | ||
|
|
||
| return; | ||
| } | ||
|
Comment on lines
+124
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The validation |
||
|
|
||
| const compressor = | ||
| type === 'gzip' | ||
| ? zlib.createGzip() | ||
| : type === 'deflate' | ||
| ? zlib.createDeflate() | ||
| : zlib.createBrotliCompress(); | ||
|
|
||
| const compressedFileName = `${path.basename(filename)}.${extMap[type]}`; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When building the compressed filename you use |
||
|
|
||
| res.statusCode = 200; | ||
| res.setHeader('Content-Type', 'application/octet-stream'); | ||
|
|
||
| res.setHeader( | ||
| 'Content-Disposition', | ||
| `attachment; filename=${compressedFileName}`, | ||
|
Comment on lines
+143
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please quote the filename in 'Content-Disposition',
`attachment; filename=${compressedFileName}`,to 'Content-Disposition',
`attachment; filename="${compressedFileName}"`,This prevents clients from mis-parsing the header and ensures the correct filename is presented to the user.
Comment on lines
+143
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| ); | ||
|
|
||
| const stream = Readable.from([fileBuffer]); | ||
|
|
||
| stream | ||
| .pipe(compressor) | ||
| .pipe(res) | ||
| .on('error', () => { | ||
| res.statusCode = 500; | ||
| res.end('Compression error'); | ||
| }); | ||
|
Comment on lines
+150
to
+156
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error handling for stream piping is fragile. Instead of chaining |
||
| }); | ||
| } | ||
|
|
||
| module.exports = { | ||
| createServer, | ||
| }; | ||
| module.exports = { createServer }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Document</title> | ||
| </head> | ||
| <body> | ||
| <main> | ||
| <h1>File Compression</h1> | ||
|
|
||
| <form action="/compress" method="post" enctype="multipart/form-data"> | ||
| <label for="file">Select a file: </label> | ||
| <input type="file" name="file" id="file" required> | ||
|
|
||
| <br> | ||
| <br> | ||
|
|
||
| <label for="type">Select a compression type: </label> | ||
| <select name="compressionType" id="type" required> | ||
| <option value="gzip">gzip</option> | ||
| <option value="deflate">deflate</option> | ||
| <option value="br">br</option> | ||
| </select> | ||
|
|
||
| <br> | ||
| <br> | ||
|
|
||
| <button type="submit">Submit</button> | ||
| </form> | ||
| </main> | ||
|
|
||
| </body> | ||
| </html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect extension mapping — the
extMapcurrently maps each type to the algorithm name ('gzip','deflate','br') so the produced filenames will befile.txt.gziporfile.txt.deflatewhich violates the requirement that the compressed file name must append these exact extensions:gzip -> .gz,deflate -> .dfl,br -> .br(description). Update the map to use the required extensions, for example:This change is required so that a request with
gzipreturnsfile.txt.gzas specified in the task.