check existence of value in header before trim process json prc (#22116)

Made extra tests to validate that `parseJsonRPCHeadersAndData` works as expected and uncovered a bug. Added check to see if the value is null before trim.
microsoft · Sep 29, 2023 · 66c7db6 · 66c7db6
1 parent e87a83c
commit 66c7db6
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 53 deletions.
diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts
@@ -114,7 +114,7 @@ export function parseJsonRPCHeadersAndData(rawData: string): ParsedRPCHeadersAnd
             break;
         }
         const [key, value] = line.split(':');
-        if (value.trim()) {
+        if (value && value.trim()) {
             if ([JSONRPC_UUID_HEADER, JSONRPC_CONTENT_LENGTH_HEADER, JSONRPC_CONTENT_TYPE_HEADER].includes(key)) {
                 headerMap.set(key.trim(), value.trim());
             }

diff --git a/src/test/testing/testController/utils.unit.test.ts b/src/test/testing/testController/utils.unit.test.ts
@@ -65,59 +65,97 @@ suite('Test Controller Utils: JSON RPC', () => {
         assert.deepStrictEqual(rpcContent.extractedJSON, json);
         assert.deepStrictEqual(rpcContent.remainingRawData, rawDataString);
     });
-});
 
-suite('Test Controller Utils: Other', () => {
-    interface TestCase {
-        name: string;
-        input: string;
-        expectedParent: string;
-        expectedSubtest: string;
-    }
-
-    const testCases: Array<TestCase> = [
-        {
-            name: 'Single parameter, named',
-            input: 'test_package.ClassName.test_method (param=value)',
-            expectedParent: 'test_package.ClassName.test_method',
-            expectedSubtest: '(param=value)',
-        },
-        {
-            name: 'Single parameter, unnamed',
-            input: 'test_package.ClassName.test_method [value]',
-            expectedParent: 'test_package.ClassName.test_method',
-            expectedSubtest: '[value]',
-        },
-        {
-            name: 'Multiple parameters, named',
-            input: 'test_package.ClassName.test_method (param1=value1, param2=value2)',
-            expectedParent: 'test_package.ClassName.test_method',
-            expectedSubtest: '(param1=value1, param2=value2)',
-        },
-        {
-            name: 'Multiple parameters, unnamed',
-            input: 'test_package.ClassName.test_method [value1, value2]',
-            expectedParent: 'test_package.ClassName.test_method',
-            expectedSubtest: '[value1, value2]',
-        },
-        {
-            name: 'Names with special characters',
-            input: 'test_package.ClassName.test_method (param1=value/1, param2=value+2)',
-            expectedParent: 'test_package.ClassName.test_method',
-            expectedSubtest: '(param1=value/1, param2=value+2)',
-        },
-        {
-            name: 'Names with spaces',
-            input: 'test_package.ClassName.test_method ["a b c d"]',
-            expectedParent: 'test_package.ClassName.test_method',
-            expectedSubtest: '["a b c d"]',
-        },
-    ];
-
-    testCases.forEach((testCase) => {
-        test(`splitTestNameWithRegex: ${testCase.name}`, () => {
-            const splitResult = splitTestNameWithRegex(testCase.input);
-            assert.deepStrictEqual(splitResult, [testCase.expectedParent, testCase.expectedSubtest]);
+    test('Valid constant', async () => {
+        const data = `{"cwd": "/Users/eleanorboyd/testingFiles/inc_dec_example", "status": "success", "result": {"test_dup_class.test_a.TestSomething.test_a": {"test": "test_dup_class.test_a.TestSomething.test_a", "outcome": "success", "message": "None", "traceback": null, "subtest": null}}}`;
+        const secondPayload = `Content-Length: 270
+Content-Type: application/json
+Request-uuid: 496c86b1-608f-4886-9436-ec00538e144c
+
+${data}`;
+        const payload = `Content-Length: 270
+Content-Type: application/json
+Request-uuid: 496c86b1-608f-4886-9436-ec00538e144c
+
+${data}${secondPayload}`;
+
+        const rpcHeaders = parseJsonRPCHeadersAndData(payload);
+        assert.deepStrictEqual(rpcHeaders.headers.size, 3);
+        const rpcContent = ExtractJsonRPCData(rpcHeaders.headers.get('Content-Length'), rpcHeaders.remainingRawData);
+        assert.deepStrictEqual(rpcContent.extractedJSON, data);
+        assert.deepStrictEqual(rpcContent.remainingRawData, secondPayload);
+    });
+    test('Valid content length as only header with carriage return', async () => {
+        const payload = `Content-Length: 7
+        `;
+
+        const rpcHeaders = parseJsonRPCHeadersAndData(payload);
+        assert.deepStrictEqual(rpcHeaders.headers.size, 1);
+        const rpcContent = ExtractJsonRPCData(rpcHeaders.headers.get('Content-Length'), rpcHeaders.remainingRawData);
+        assert.deepStrictEqual(rpcContent.extractedJSON, '');
+        assert.deepStrictEqual(rpcContent.remainingRawData, '');
+    });
+    test('Valid content length header with no value', async () => {
+        const payload = `Content-Length:`;
+
+        const rpcHeaders = parseJsonRPCHeadersAndData(payload);
+        const rpcContent = ExtractJsonRPCData(rpcHeaders.headers.get('Content-Length'), rpcHeaders.remainingRawData);
+        assert.deepStrictEqual(rpcContent.extractedJSON, '');
+        assert.deepStrictEqual(rpcContent.remainingRawData, '');
+    });
+
+    suite('Test Controller Utils: Other', () => {
+        interface TestCase {
+            name: string;
+            input: string;
+            expectedParent: string;
+            expectedSubtest: string;
+        }
+
+        const testCases: Array<TestCase> = [
+            {
+                name: 'Single parameter, named',
+                input: 'test_package.ClassName.test_method (param=value)',
+                expectedParent: 'test_package.ClassName.test_method',
+                expectedSubtest: '(param=value)',
+            },
+            {
+                name: 'Single parameter, unnamed',
+                input: 'test_package.ClassName.test_method [value]',
+                expectedParent: 'test_package.ClassName.test_method',
+                expectedSubtest: '[value]',
+            },
+            {
+                name: 'Multiple parameters, named',
+                input: 'test_package.ClassName.test_method (param1=value1, param2=value2)',
+                expectedParent: 'test_package.ClassName.test_method',
+                expectedSubtest: '(param1=value1, param2=value2)',
+            },
+            {
+                name: 'Multiple parameters, unnamed',
+                input: 'test_package.ClassName.test_method [value1, value2]',
+                expectedParent: 'test_package.ClassName.test_method',
+                expectedSubtest: '[value1, value2]',
+            },
+            {
+                name: 'Names with special characters',
+                input: 'test_package.ClassName.test_method (param1=value/1, param2=value+2)',
+                expectedParent: 'test_package.ClassName.test_method',
+                expectedSubtest: '(param1=value/1, param2=value+2)',
+            },
+            {
+                name: 'Names with spaces',
+                input: 'test_package.ClassName.test_method ["a b c d"]',
+                expectedParent: 'test_package.ClassName.test_method',
+                expectedSubtest: '["a b c d"]',
+            },
+        ];
+
+        testCases.forEach((testCase) => {
+            test(`splitTestNameWithRegex: ${testCase.name}`, () => {
+                const splitResult = splitTestNameWithRegex(testCase.input);
+                assert.deepStrictEqual(splitResult, [testCase.expectedParent, testCase.expectedSubtest]);
+            });
         });
     });
 });