Fix: Change report severity for small files.

Fix webhintio#4031
sarvaje · Dec 10, 2020 · f813636 · f813636
1 parent d7fffdd
commit f813636
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 9 deletions.
diff --git a/packages/hint-http-compression/README.md b/packages/hint-http-compression/README.md
@@ -1008,10 +1008,12 @@ In the [`.hintrc`][hintrc] file:
         "http-compression": [ "warning", {
             "resource": {
                 "<compression_type>": <true|false>,
+                "threshold": <number>
                 ...
             },
             "html": {
                 "<compression_type>": <true|false>,
+                "threshold": <number>
                 ...
             }
         },
@@ -1023,7 +1025,8 @@ In the [`.hintrc`][hintrc] file:
 
 Where `<compression_method>` can be one of: `brotli`, `gzip`, or
 `zopfli`.
-
+And `threshold` is the maximum size (in bytes) to set the
+severity of a report to `hint`.
 E.g. If you want the hint to check if only the page resources are
 served compressed using Brotli, and not the page itself, you can
 use the following configuration in the [`.hintrc`][hintrc]:
@@ -1035,7 +1038,8 @@ use the following configuration in the [`.hintrc`][hintrc]:
     "hints": {
         "http-compression": [ "warning", {
             "html": {
-                "brotli": false
+                "brotli": false,
+                "threshold": 1024
             }
         }],
         ...

diff --git a/packages/hint-http-compression/src/hint.ts b/packages/hint-http-compression/src/hint.ts
@@ -47,6 +47,7 @@ export default class HttpCompressionHint implements IHint {
             return {
                 brotli: true,
                 gzip: true,
+                threshold: 1024,
                 zopfli: true,
                 ...(context.hintOptions && context.hintOptions[property])
             };
@@ -57,6 +58,10 @@ export default class HttpCompressionHint implements IHint {
 
         // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
+        const isBigFile = (size: number, options: CompressionCheckOptions) => {
+            return size > options.threshold;
+        };
+
         const checkIfBytesMatch = (rawResponse: Buffer, magicNumbers: number[]) => {
             return rawResponse && magicNumbers.every((b, i) => {
                 return rawResponse[i] === b;
@@ -281,7 +286,7 @@ export default class HttpCompressionHint implements IHint {
             return !checkIfBytesMatch(rawResponse, [0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x03]);
         };
 
-        const checkBrotli = async (resource: string, element: HTMLElement | null) => {
+        const checkBrotli = async (resource: string, element: HTMLElement | null, options: CompressionCheckOptions) => {
             let networkData = await getNetworkData(resource, { 'Accept-Encoding': 'br' });
 
             if (!networkData) {
@@ -340,7 +345,7 @@ export default class HttpCompressionHint implements IHint {
                 context.report(
                     resource,
                     getMessage('compressedWithBrotliOverHTTPS', context.language),
-                    { element, severity: Severity.warning }
+                    { element, severity: isBigFile(rawResponse.byteLength, options) ? Severity.warning : Severity.hint }
                 );
 
                 return;
@@ -427,7 +432,7 @@ export default class HttpCompressionHint implements IHint {
                 context.report(
                     resource,
                     generateGzipCompressionMessage('gzip'),
-                    { element, severity: Severity.error });
+                    { element, severity: isBigFile(rawResponse.byteLength, shouldCheckIfCompressedWith) ? Severity.error : Severity.hint });
 
                 return;
             }
@@ -844,7 +849,7 @@ export default class HttpCompressionHint implements IHint {
             }
 
             if (shouldCheckIfCompressedWith.brotli) {
-                await checkBrotli(resource, element);
+                await checkBrotli(resource, element, shouldCheckIfCompressedWith);
             }
         };
 

diff --git a/packages/hint-http-compression/src/types.ts b/packages/hint-http-compression/src/types.ts
@@ -1,5 +1,6 @@
 export type CompressionCheckOptions = {
     brotli: boolean;
     gzip: boolean;
+    threshold: number; // bytes
     zopfli: boolean;
 };
diff --git a/packages/hint-http-compression/tests/_tests.ts b/packages/hint-http-compression/tests/_tests.ts
@@ -248,6 +248,17 @@ const testsForBrotli: HintTest[] = [
             scriptFileHeaders: { 'Content-Encoding': null }
         })
     },
+    {
+        name: `Resource is not served compressed with Brotli when Brotli compression is requested but it is a very small file`,
+        reports: [{
+            message: generateCompressionMessage('Brotli', false, 'over HTTPS'),
+            severity: Severity.hint
+        }],
+        serverConfig: createBrotliServerConfig({
+            scriptFileContent: scriptSmallFile.original,
+            scriptFileHeaders: { 'Content-Encoding': null }
+        })
+    },
     {
         name: `Resource is served compressed with Brotli and without the 'Content-Encoding' header when Brotli compression is requested`,
         reports: [{
@@ -445,6 +456,29 @@ const testsForDisallowedCompressionMethods = (https: boolean = false): HintTest[
                 https
             )
         },
+        {
+            name: `Compressed resource is served with disallowed 'Content-Encoding: x-compress' header but it is a very small file`,
+            reports: [
+                {
+                    message: generateDisallowedCompressionMessage('x-compress'),
+                    severity: Severity.warning
+                },
+                {
+                    message: generateCompressionMessage('gzip'),
+                    severity: Severity.hint
+                }
+            ],
+            serverConfig: createGzipZopfliServerConfig(
+                {
+                    scriptFileContent: scriptSmallFile.original,
+                    scriptFileHeaders: {
+                        'Content-Encoding': 'x-compress',
+                        vary: 'Accept-Encoding'
+                    }
+                },
+                https
+            )
+        },
         {
             name: `Compressed resource is served with 'Get-Dictionary' header`,
             reports: [{
@@ -482,6 +516,21 @@ const testsForGzipZopfli = (https: boolean = false): HintTest[] => {
                 https
             )
         },
+        {
+            name: `Resource is not served compressed with gzip when gzip compression is requested but it is a very small file`,
+            reports: [{
+                message: generateCompressionMessage('gzip'),
+                severity: Severity.hint
+            }],
+            serverConfig: createGzipZopfliServerConfig(
+                {
+                    request: { headers: { 'Accept-Encoding': 'gzip' } },
+                    scriptFileContent: scriptSmallFile.original,
+                    scriptFileHeaders: { 'Content-Encoding': null }
+                },
+                https
+            )
+        },
         {
             name: `Resource is served compressed with gzip and without the 'Content-Encoding' header when gzip compression is requested`,
             reports: [

diff --git a/packages/hint-http-compression/tests/tests-http.ts b/packages/hint-http-compression/tests/tests-http.ts
@@ -37,14 +37,14 @@ testHint(hintPath, testsForGzipZopfliUASniffing(), testConfigs);
 testHint(hintPath, testsForBrotliOverHTTP, testConfigs);
 
 // Tests for the user options.
-[true, false].forEach((isTarget) => {
+[true, false].forEach((isHTML) => {
     ['gzip', 'zopfli', 'brotli'].forEach((encoding) => {
         testHint(
             hintPath,
-            testsForUserConfigs(`${encoding}`, isTarget),
+            testsForUserConfigs(`${encoding}`, isHTML),
             {
                 ...testConfigs,
-                hintOptions: { [isTarget ? 'target' : 'resource']: { [encoding]: false } }
+                hintOptions: { [isHTML ? 'html' : 'resource']: { [encoding]: false } }
             }
         );
     });