Skip to content

Commit 44064f0

Browse files
authored
Validate query type in the ResultSummary (#869)
Unexpected query types are protocol violations and it should be treat like it.
1 parent b99d313 commit 44064f0

File tree

3 files changed

+61
-5
lines changed

3 files changed

+61
-5
lines changed

packages/bolt-connection/src/bolt/stream-observers.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,13 +240,22 @@ class ResultStreamObserver extends StreamObserver {
240240
}
241241

242242
_handlePullSuccess (meta) {
243-
this._setState(_states.SUCCEEDED)
244243
const completionMetadata = Object.assign(
245244
this._server ? { server: this._server } : {},
246245
this._meta,
247246
meta
248247
)
249248

249+
if (![undefined, null, 'r', 'w', 'rw', 's'].includes(completionMetadata.type)) {
250+
this.onError(
251+
newError(
252+
`Server returned invalid query type. Expected one of [undefined, null, "r", "w", "rw", "s"] but got '${completionMetadata.type}'`,
253+
PROTOCOL_ERROR))
254+
return
255+
}
256+
257+
this._setState(_states.SUCCEEDED)
258+
250259
let beforeHandlerResult = null
251260
if (this._beforeComplete) {
252261
beforeHandlerResult = this._beforeComplete(completionMetadata)

packages/bolt-connection/test/bolt/stream-observer.test.js

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,57 @@ describe('#unit ResultStreamObserver', () => {
511511
})
512512
})
513513
})
514+
515+
describe('metadata validation', () => {
516+
it.each([
517+
['wr'],
518+
['read'],
519+
['read/write'],
520+
['write/read'],
521+
['write'],
522+
['undefined'],
523+
['null'],
524+
['banana']
525+
])(`should trigger onError when the type is '%s'`, (type) => {
526+
const streamObserver = newStreamObserver()
527+
const expectedError = newError(
528+
`Server returned invalid query type. Expected one of [undefined, null, "r", "w", "rw", "s"] but got '${type}'`,
529+
PROTOCOL_ERROR)
530+
531+
streamObserver.onCompleted({ fields: ['A', 'B', 'C'] })
532+
streamObserver.onCompleted({ type })
533+
534+
streamObserver.subscribe(
535+
newObserver(NO_OP, receivedError => {
536+
expect(receivedError).toEqual(expectedError)
537+
}, () => {
538+
fail('Should not succeed')
539+
})
540+
)
541+
})
542+
543+
it.each([
544+
['r'],
545+
['w'],
546+
['rw'],
547+
['s'],
548+
[null],
549+
[undefined]
550+
])(`should trigger onComplete when the type is '%s'`, (type) => {
551+
const streamObserver = newStreamObserver()
552+
553+
streamObserver.onCompleted({ fields: ['A', 'B', 'C'] })
554+
streamObserver.onCompleted({ type })
555+
556+
streamObserver.subscribe(
557+
newObserver(NO_OP, () => {
558+
fail('should not fail')
559+
}, meta => {
560+
expect(meta.type).toBe(type)
561+
})
562+
)
563+
})
564+
})
514565
})
515566

516567
describe('#unit RouteObserver', () => {

packages/testkit-backend/src/skipped-tests/common.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,6 @@ const skippedTests = [
2727
'Flaky in TeamCity',
2828
ifEndsWith('test_should_fail_when_writing_to_unexpectedly_interrupting_writers_on_run_using_tx_function'),
2929
),
30-
skip(
31-
'Driver does not validate query type values in result summaries',
32-
ifEquals('stub.summary.test_summary.TestSummary.test_invalid_query_type')
33-
),
3430
skip(
3531
'ResultSummary.notifications defaults to empty array instead of return null/undefined',
3632
ifEquals('stub.summary.test_summary.TestSummary.test_no_notifications'),

0 commit comments

Comments
 (0)