From: Chocobozzz <me@florianbigard.com>
Date: Thu, 21 Feb 2019 16:19:16 +0000 (+0100)
Subject: Add request body limit
X-Git-Tag: v1.3.0-rc.1~156
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=bfe2ef6bfae03444a232883fc7c449206cf3bee4;p=oweals%2Fpeertube.git

Add request body limit
---

diff --git a/server/helpers/requests.ts b/server/helpers/requests.ts
index 5c6dc5e19..3762e4d3c 100644
--- a/server/helpers/requests.ts
+++ b/server/helpers/requests.ts
@@ -1,12 +1,14 @@
 import * as Bluebird from 'bluebird'
-import { createWriteStream } from 'fs-extra'
+import { createWriteStream, remove } from 'fs-extra'
 import * as request from 'request'
 import { ACTIVITY_PUB, CONFIG } from '../initializers'
 import { processImage } from './image-utils'
 import { join } from 'path'
+import { logger } from './logger'
 
 function doRequest <T> (
-  requestOptions: request.CoreOptions & request.UriOptions & { activityPub?: boolean }
+  requestOptions: request.CoreOptions & request.UriOptions & { activityPub?: boolean },
+  bodyKBLimit = 1000 // 1MB
 ): Bluebird<{ response: request.RequestResponse, body: T }> {
   if (requestOptions.activityPub === true) {
     if (!Array.isArray(requestOptions.headers)) requestOptions.headers = {}
@@ -15,16 +17,29 @@ function doRequest <T> (
 
   return new Bluebird<{ response: request.RequestResponse, body: T }>((res, rej) => {
     request(requestOptions, (err, response, body) => err ? rej(err) : res({ response, body }))
+      .on('data', onRequestDataLengthCheck(bodyKBLimit))
   })
 }
 
-function doRequestAndSaveToFile (requestOptions: request.CoreOptions & request.UriOptions, destPath: string) {
+function doRequestAndSaveToFile (
+  requestOptions: request.CoreOptions & request.UriOptions,
+  destPath: string,
+  bodyKBLimit = 10000 // 10MB
+) {
   return new Bluebird<void>((res, rej) => {
     const file = createWriteStream(destPath)
     file.on('finish', () => res())
 
     request(requestOptions)
-      .on('error', err => rej(err))
+      .on('data', onRequestDataLengthCheck(bodyKBLimit))
+      .on('error', err => {
+        file.close()
+
+        remove(destPath)
+          .catch(err => logger.error('Cannot remove %s after request failure.', destPath, { err }))
+
+        return rej(err)
+      })
       .pipe(file)
   })
 }
@@ -44,3 +59,21 @@ export {
   doRequestAndSaveToFile,
   downloadImage
 }
+
+// ---------------------------------------------------------------------------
+
+// Thanks to https://github.com/request/request/issues/2470#issuecomment-268929907 <3
+function onRequestDataLengthCheck (bodyKBLimit: number) {
+  let bufferLength = 0
+  const bytesLimit = bodyKBLimit * 1000
+
+  return function (chunk) {
+    bufferLength += chunk.length
+    if (bufferLength > bytesLimit) {
+      this.abort()
+
+      const error = new Error(`Response was too large - aborted after ${bytesLimit} bytes.`)
+      this.emit('error', error)
+    }
+  }
+}
diff --git a/server/lib/hls.ts b/server/lib/hls.ts
index 3575981f4..16a805ac2 100644
--- a/server/lib/hls.ts
+++ b/server/lib/hls.ts
@@ -116,7 +116,8 @@ function downloadPlaylistSegments (playlistUrl: string, destinationDir: string,
       for (const fileUrl of fileUrls) {
         const destPath = join(tmpDirectory, basename(fileUrl))
 
-        await doRequestAndSaveToFile({ uri: fileUrl }, destPath)
+        const bodyKBLimit = 10 * 1000 * 1000 // 10GB
+        await doRequestAndSaveToFile({ uri: fileUrl }, destPath, bodyKBLimit)
       }
 
       clearTimeout(timer)
diff --git a/server/tests/helpers/index.ts b/server/tests/helpers/index.ts
index 551208245..03b971770 100644
--- a/server/tests/helpers/index.ts
+++ b/server/tests/helpers/index.ts
@@ -1,2 +1,3 @@
 import './core-utils'
 import './comment-model'
+import './request'
diff --git a/server/tests/helpers/request.ts b/server/tests/helpers/request.ts
new file mode 100644
index 000000000..95a74fdfa
--- /dev/null
+++ b/server/tests/helpers/request.ts
@@ -0,0 +1,48 @@
+/* tslint:disable:no-unused-expression */
+
+import 'mocha'
+import { doRequest, doRequestAndSaveToFile } from '../../helpers/requests'
+import { get4KFileUrl, root, wait } from '../../../shared/utils'
+import { join } from 'path'
+import { pathExists, remove } from 'fs-extra'
+import { expect } from 'chai'
+
+describe('Request helpers', function () {
+  const destPath1 = join(root(), 'test-output-1.txt')
+  const destPath2 = join(root(), 'test-output-2.txt')
+
+  it('Should throw an error when the bytes limit is exceeded for request', async function () {
+    try {
+      await doRequest({ uri: get4KFileUrl() }, 3)
+    } catch {
+      return
+    }
+
+    throw new Error('No error thrown by do request')
+  })
+
+  it('Should throw an error when the bytes limit is exceeded for request and save file', async function () {
+    try {
+      await doRequestAndSaveToFile({ uri: get4KFileUrl() }, destPath1, 3)
+    } catch {
+
+      await wait(500)
+      expect(await pathExists(destPath1)).to.be.false
+      return
+    }
+
+    throw new Error('No error thrown by do request and save to file')
+  })
+
+  it('Should succeed if the file is below the limit', async function () {
+    await doRequest({ uri: get4KFileUrl() }, 5)
+    await doRequestAndSaveToFile({ uri: get4KFileUrl() }, destPath2, 5)
+
+    expect(await pathExists(destPath2)).to.be.true
+  })
+
+  after(async function () {
+    await remove(destPath1)
+    await remove(destPath2)
+  })
+})
diff --git a/shared/utils/requests/requests.ts b/shared/utils/requests/requests.ts
index 6b59e24fc..dc2d4abe5 100644
--- a/shared/utils/requests/requests.ts
+++ b/shared/utils/requests/requests.ts
@@ -3,6 +3,10 @@ import { buildAbsoluteFixturePath, root } from '../miscs/miscs'
 import { isAbsolute, join } from 'path'
 import { parse } from 'url'
 
+function get4KFileUrl () {
+  return 'https://download.cpy.re/peertube/4k_file.txt'
+}
+
 function makeRawRequest (url: string, statusCodeExpected?: number, range?: string) {
   const { host, protocol, pathname } = parse(url)
 
@@ -166,6 +170,7 @@ function updateAvatarRequest (options: {
 // ---------------------------------------------------------------------------
 
 export {
+  get4KFileUrl,
   makeHTMLRequest,
   makeGetRequest,
   makeUploadRequest,