From 64d57792c776760fee86472df052321ea56cf1e2 Mon Sep 17 00:00:00 2001
From: Antonio Barcelos <antonio.barcelos@neo4j.com>
Date: Tue, 8 Feb 2022 11:53:56 +0100
Subject: [PATCH] Validate query type in the ResultSummary

Unexpected query types are protocol violations and it should be treaten like it.
---
 .../src/bolt/stream-observers.js              | 11 +++-
 .../test/bolt/stream-observer.test.js         | 51 +++++++++++++++++++
 .../src/skipped-tests/common.js               |  4 --
 3 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/packages/bolt-connection/src/bolt/stream-observers.js b/packages/bolt-connection/src/bolt/stream-observers.js
index b71411ea6..518281dd4 100644
--- 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
index ee018b5fe..13eeb24a6 100644
--- 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
index 38b78f115..6af9f7b49 100644
--- 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'),