Validate query type in the ResultSummary (#869)

bigmontz · web-flow · commit 44064f05c5f6 · 2022-02-08T16:45:02.000+01:00
Unexpected query types are protocol violations and it should be treat like it.
diff --git a/packages/bolt-connection/src/bolt/stream-observers.js b/packages/bolt-connection/src/bolt/stream-observers.js
@@ -240,13 +240,22 @@ class ResultStreamObserver extends StreamObserver {
   }
 
   _handlePullSuccess (meta) {
-    this._setState(_states.SUCCEEDED)
     const completionMetadata = Object.assign(
       this._server ? { server: this._server } : {},
       this._meta,
       meta
     )
 
+    if (![undefined, null, 'r', 'w', 'rw', 's'].includes(completionMetadata.type)) {
+      this.onError(
+        newError(
+          `Server returned invalid query type. Expected one of [undefined, null, "r", "w", "rw", "s"] but got '${completionMetadata.type}'`,
+          PROTOCOL_ERROR))
+      return
+    }
+
+    this._setState(_states.SUCCEEDED)
+
     let beforeHandlerResult = null
     if (this._beforeComplete) {
       beforeHandlerResult = this._beforeComplete(completionMetadata)
diff --git a/packages/bolt-connection/test/bolt/stream-observer.test.js b/packages/bolt-connection/test/bolt/stream-observer.test.js
@@ -511,6 +511,57 @@ describe('#unit ResultStreamObserver', () => {
       })
     })
   })
+
+  describe('metadata validation', () => {
+    it.each([
+      ['wr'],
+      ['read'],
+      ['read/write'],
+      ['write/read'],
+      ['write'],
+      ['undefined'],
+      ['null'],
+      ['banana']
+    ])(`should trigger onError when the type is '%s'`, (type) => {
+      const streamObserver = newStreamObserver()
+      const expectedError = newError(
+        `Server returned invalid query type. Expected one of [undefined, null, "r", "w", "rw", "s"] but got '${type}'`,
+        PROTOCOL_ERROR)
+
+      streamObserver.onCompleted({ fields: ['A', 'B', 'C'] })
+      streamObserver.onCompleted({ type })
+
+      streamObserver.subscribe(
+        newObserver(NO_OP, receivedError => {
+          expect(receivedError).toEqual(expectedError)
+        }, () => {
+          fail('Should not succeed')
+        })
+      )
+    })
+
+    it.each([
+      ['r'],
+      ['w'],
+      ['rw'],
+      ['s'],
+      [null],
+      [undefined]
+    ])(`should trigger onComplete when the type is '%s'`, (type) => {
+      const streamObserver = newStreamObserver()
+
+      streamObserver.onCompleted({ fields: ['A', 'B', 'C'] })
+      streamObserver.onCompleted({ type })
+
+      streamObserver.subscribe(
+        newObserver(NO_OP, () => {
+          fail('should not fail')
+        }, meta => {
+          expect(meta.type).toBe(type)
+        })
+      )
+    })
+  })
 })
 
 describe('#unit RouteObserver', () => {
diff --git a/packages/testkit-backend/src/skipped-tests/common.js b/packages/testkit-backend/src/skipped-tests/common.js
@@ -27,10 +27,6 @@ const skippedTests = [
     'Flaky in TeamCity',
     ifEndsWith('test_should_fail_when_writing_to_unexpectedly_interrupting_writers_on_run_using_tx_function'),
   ),
-  skip(
-    'Driver does not validate query type values in result summaries',
-    ifEquals('stub.summary.test_summary.TestSummary.test_invalid_query_type')
-  ),
   skip(
     'ResultSummary.notifications defaults to empty array instead of return null/undefined',
     ifEquals('stub.summary.test_summary.TestSummary.test_no_notifications'),
