From 68d65bf92bb4539187dbc092f4c5515b817f7062 Mon Sep 17 00:00:00 2001
From: Richard Petersen <richard.petersen@open-xchange.com>
Date: Tue, 6 Sep 2022 10:47:42 +0200
Subject: [PATCH] Introduce contstraints, when to use gzip

---
 .env.defaults                                 |  2 +
 README.md                                     | 43 +++++++++--------
 .../templates/deployment.yaml                 |  4 ++
 helm/core-ui-middleware/values.yaml           |  2 +
 integration/caching_test.js                   |  5 +-
 spec/file_caching_test.js                     | 46 ++++++++++++++++++-
 src/files.js                                  | 22 ++++++---
 7 files changed, 93 insertions(+), 31 deletions(-)

diff --git a/.env.defaults b/.env.defaults
index d25e90d..eceb03d 100644
--- a/.env.defaults
+++ b/.env.defaults
@@ -4,6 +4,8 @@ PORT=8080
 LOG_LEVEL=info
 APP_ROOT=/
 EXPOSE_API_DOCS=false
+COMPRESS_FILE_SIZE=600
+COMPRESS_FILE_TYPES=application/javascript application/json application/x-javascript application/xml application/xml+rss text/css text/html text/javascript text/plain text/xml image/svg+xml
 
 REDIS_PORT=6379
 REDIS_DB=0
diff --git a/README.md b/README.md
index 0f128cf..b3be80a 100644
--- a/README.md
+++ b/README.md
@@ -37,28 +37,33 @@ It is possible to horizontally scale the UI Middleware, as more clients are fetc
 
 **local, docker**
 
-| Parameter       | Description                     | Default  |
-|-----------------|---------------------------------|----------|
-| `PORT`          | Exposed port                    | `"8080"` |
-| `CACHE_TTL`     | Vite manifest caching time      | `30000`  |
-| `LOG_LEVEL`     | Pino log level                  | `"info"` |
-| `REDIS_HOST`    | Redis host (required)           |          |
-| `REDIS_PORT`    | Redis port (optional)           | `6379`   |
-| `REDIS_DB`      | Redis DB, e.g. `"1"` (optional) | null     |
-| `REDIS_PASSWORD`| Redis password (optional)       | null     |
+| Parameter             | Description                     | Default  |
+|-----------------------|---------------------------------|----------|
+| `PORT`                | Exposed port                    | `"8080"` |
+| `CACHE_TTL`           | Vite manifest caching time      | `30000`  |
+| `LOG_LEVEL`           | Pino log level                  | `"info"` |
+| `REDIS_HOST`          | Redis host (required)           |          |
+| `REDIS_PORT`          | Redis port (optional)           | `6379`   |
+| `REDIS_DB`            | Redis DB, e.g. `"1"` (optional) | null     |
+| `REDIS_PASSWORD`      | Redis password (optional)       | null     |
+| `COMPRESS_FILE_SIZE`  | Larger files will be gzipped    | `600`    |
+| `COMPRESS_FILE_TYPES` | Set of compression mime types   |see values|
+
 
 **kubernetes**
 
-| Parameter       | Description                     | Default  |
-|-----------------|---------------------------------|----------|
-| `port`          | Exposed port                    | `"8080"` |
-| `cacheTTL`      | Vite manifest caching time      | `30000`  |
-| `logLevel`      | Pino log level                  | `"info"` |
-| `redis.enabled` | Global switch Redis integration |  false   |
-| `redis.host`    | Redis host                      |          |
-| `redis.port`    | Redis port (optional)           | `6379`   |
-| `redis.db`      | Redis DB, e.g. `"1"` (optional) | null     |
-| `redis.password`| Redis password (optional)       | null     |
+| Parameter             | Description                     | Default  |
+|-----------------------|---------------------------------|----------|
+| `port`                | Exposed port                    | `"8080"` |
+| `cacheTTL`            | Vite manifest caching time      | `30000`  |
+| `logLevel`            | Pino log level                  | `"info"` |
+| `redis.enabled`       | Global switch Redis integration |  false   |
+| `redis.host`          | Redis host                      |          |
+| `redis.port`          | Redis port (optional)           | `6379`   |
+| `redis.db`            | Redis DB, e.g. `"1"` (optional) | null     |
+| `redis.password`      | Redis password (optional)       | null     |
+| `compressFileSize`    | Larger files will be gzipped    | `600`    |
+| `compressFileTypes`   | Set of compression mime types   |see values|
 
 ## Ingress
 
diff --git a/helm/core-ui-middleware/templates/deployment.yaml b/helm/core-ui-middleware/templates/deployment.yaml
index f3508e5..09acd67 100644
--- a/helm/core-ui-middleware/templates/deployment.yaml
+++ b/helm/core-ui-middleware/templates/deployment.yaml
@@ -27,6 +27,10 @@ spec:
               value: "{{ .Values.logLevel }}"
             - name: APP_ROOT
               value: "{{ .Values.appRoot }}"
+            - name: COMPRESS_FILE_SIZE
+              value: "{{ .Values.compressFileSize }}"
+            - name: COMPRESS_FILE_TYPES
+              value: "{{ .Values.compressFileTypes }}"
             {{- if .Values.redis.enabled }}
             - name: REDIS_HOST
               value: "{{ required "redis.host required" .Values.redis.host }}"
diff --git a/helm/core-ui-middleware/values.yaml b/helm/core-ui-middleware/values.yaml
index 37f16be..3423015 100644
--- a/helm/core-ui-middleware/values.yaml
+++ b/helm/core-ui-middleware/values.yaml
@@ -103,6 +103,8 @@ cacheTTL: 30000
 logLevel: info
 baseUrls: []
 appRoot: '/'
+compressFileSize: 600
+compressFileTypes: application/javascript application/json application/x-javascript application/xml application/xml+rss text/css text/html text/javascript text/plain text/xml image/svg+xml
 
 redis:
   enabled: false
diff --git a/integration/caching_test.js b/integration/caching_test.js
index e500580..6705685 100644
--- a/integration/caching_test.js
+++ b/integration/caching_test.js
@@ -4,7 +4,6 @@ import { generateSimpleViteManifest, mockApp, mockConfig, mockFetch } from '../s
 import { client } from '../src/redis.js'
 import * as td from 'testdouble'
 import { getRedisKey } from '../src/util.js'
-import { gunzipSync } from 'node:zlib'
 
 describe('File caching service', function () {
   let app
@@ -42,9 +41,9 @@ describe('File caching service', function () {
     const version = response.headers.version
 
     const body = (await client.getBuffer(getRedisKey({ version, name: '/index.html:body' }))) || ''
-    expect(gunzipSync(body).toString()).to.equal('<html><head></head><body>it\'s me</body></html>')
+    expect(body.toString()).to.equal('<html><head></head><body>it\'s me</body></html>')
     const meta = await client.get(getRedisKey({ version, name: '/index.html:meta' }))
-    expect(meta).to.equal('{"headers":{"content-type":"text/html","dependencies":false,"content-encoding":"gzip"}}')
+    expect(meta).to.equal('{"headers":{"content-type":"text/html","dependencies":false}}')
   })
 
   it('serves files from redis and stores them in local cache', async function () {
diff --git a/spec/file_caching_test.js b/spec/file_caching_test.js
index 229cd6f..ce832a1 100644
--- a/spec/file_caching_test.js
+++ b/spec/file_caching_test.js
@@ -350,11 +350,53 @@ describe('File caching service', function () {
   })
 
   it('serves files as gzip', async function () {
-    const response = await request(app.server).get('/example.js')
+    mockFetch({
+      'http://ui-server': {
+        '/manifest.json': generateSimpleViteManifest({}),
+        '/large.js': () => new Response([...new Array(2500)].join(' '), { headers: { 'content-type': 'application/javascript' } })
+      }
+    })
+    app = await mockApp()
+
+    const response = await request(app.server).get('/large.js')
     expect(response.statusCode).to.equal(200)
     expect(response.headers['content-encoding']).to.equal('gzip')
 
     // check for files in redis
-    expect(zlib.gunzipSync(await redis.client.getBuffer('ui-middleware:554855300:/example.js:body')).toString()).to.equal(response.text)
+    expect(zlib.gunzipSync(await redis.client.getBuffer('ui-middleware:554855300:/large.js:body')).toString()).to.equal(response.text)
+  })
+
+  it('does not serve small files with gzip', async function () {
+    mockFetch({
+      'http://ui-server': {
+        '/manifest.json': generateSimpleViteManifest({}),
+        '/small.js': () => new Response('small', { headers: { 'content-type': 'application/javascript' } })
+      }
+    })
+    app = await mockApp()
+
+    const response = await request(app.server).get('/small.js')
+    expect(response.statusCode).to.equal(200)
+    expect(response.headers).to.not.have.property('content-encoding')
+
+    // check for files in redis
+    expect((await redis.client.getBuffer('ui-middleware:554855300:/small.js:body')).toString()).to.equal(response.text)
+  })
+
+  it('does not serve other mime types with gzip', async function () {
+    mockFetch({
+      'http://ui-server': {
+        '/manifest.json': generateSimpleViteManifest({}),
+        '/file.mp3': () => new Response('123', { headers: { 'content-type': 'audio/mpeg' } })
+      }
+    })
+    app = await mockApp()
+
+    const response = await request(app.server).get('/file.mp3')
+    expect(response.statusCode).to.equal(200)
+    expect(response.headers).to.not.have.property('content-encoding')
+
+    // check for files in redis
+    expect(await redis.client.getBuffer('ui-middleware:554855300:/file.mp3:body')).to.deep.equal(response.body)
   })
 })
diff --git a/src/files.js b/src/files.js
index e7bd62a..107ea8e 100644
--- a/src/files.js
+++ b/src/files.js
@@ -9,6 +9,10 @@ import zlib from 'node:zlib'
 import { promisify } from 'node:util'
 const gzip = promisify(zlib.gzip)
 
+const compressFileSize = Number(process.env.COMPRESS_FILE_SIZE)
+const compressionMimeTypes = (process.env.COMPRESS_FILE_TYPES || '').split(' ')
+const compressionWhitelistRegex = new RegExp(`^(${compressionMimeTypes.join('|')})$`)
+
 export function createWritable (body) {
   if (typeof body !== 'string' && !(body instanceof Buffer)) return JSON.stringify(body)
   return body
@@ -24,7 +28,7 @@ async function createFileBuffer (response, dependencies) {
   buffer.fill(Buffer.from(resBuffer), 0, resBuffer.byteLength)
   if (appendix) buffer.write(appendix, resBuffer.byteLength)
 
-  return await gzip(buffer)
+  return buffer
 }
 
 export async function fetchFileWithHeadersFromBaseUrl (path, baseUrl, version) {
@@ -35,16 +39,20 @@ export async function fetchFileWithHeadersFromBaseUrl (path, baseUrl, version) {
 
   if (!response.ok) throw new NotFoundError(`Error fetching file: ${path}`)
 
-  const body = await createFileBuffer(response, dependencies)
-
-  return {
-    body,
+  const result = {
+    body: await createFileBuffer(response, dependencies),
     headers: {
       'content-type': response.headers.get('content-type'),
-      dependencies,
-      'content-encoding': 'gzip'
+      dependencies
     }
   }
+
+  if (result.body.length > compressFileSize && compressionWhitelistRegex.test(result.headers['content-type'])) {
+    result.body = await gzip(result.body)
+    result.headers['content-encoding'] = 'gzip'
+  }
+
+  return result
 }
 
 export async function fetchFileWithHeaders ({ path, version }) {
-- 
GitLab