Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 26f0aeb

Browse filesBrowse files
committed
Eliminate false positives by detecting non-stream objects returned from pipe() calls based on accessed properties
1 parent b6b64d2 commit 26f0aeb
Copy full SHA for 26f0aeb

File tree

3 files changed

+36
-3
lines changed
Filter options

3 files changed

+36
-3
lines changed

‎javascript/ql/src/Quality/UnhandledStreamPipe.ql

Copy file name to clipboardExpand all lines: javascript/ql/src/Quality/UnhandledStreamPipe.ql
+35-1Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,21 @@ string getNonchainableStreamMethodName() {
4848
result = ["read", "write", "end", "pipe", "unshift", "push", "isPaused", "wrap", "emit"]
4949
}
5050

51+
/**
52+
* Gets the property names commonly found on Node.js streams.
53+
*/
54+
string getStreamPropertyName() {
55+
result =
56+
[
57+
"readable", "writable", "destroyed", "closed", "readableHighWaterMark", "readableLength",
58+
"readableObjectMode", "readableEncoding", "readableFlowing", "readableEnded", "flowing",
59+
"writableHighWaterMark", "writableLength", "writableObjectMode", "writableFinished",
60+
"writableCorked", "writableEnded", "defaultEncoding", "allowHalfOpen", "objectMode",
61+
"errored", "pending", "autoDestroy", "encoding", "path", "fd", "bytesRead", "bytesWritten",
62+
"_readableState", "_writableState"
63+
]
64+
}
65+
5166
/**
5267
* Gets all method names commonly found on Node.js streams.
5368
*/
@@ -109,6 +124,25 @@ predicate isPipeFollowedByNonStreamMethod(PipeCall pipeCall) {
109124
)
110125
}
111126

127+
/**
128+
* Holds if the pipe call result is used to access a property that is not typical of streams.
129+
*/
130+
predicate isPipeFollowedByNonStreamProperty(PipeCall pipeCall) {
131+
exists(DataFlow::PropRef propRef |
132+
propRef = pipeResultRef(pipeCall).getAPropertyRead() and
133+
not propRef.getPropertyName() = getStreamPropertyName()
134+
)
135+
}
136+
137+
/**
138+
* Holds if the pipe call result is used in a non-stream-like way,
139+
* either by calling non-stream methods or accessing non-stream properties.
140+
*/
141+
predicate isPipeFollowedByNonStreamAccess(PipeCall pipeCall) {
142+
isPipeFollowedByNonStreamMethod(pipeCall) or
143+
isPipeFollowedByNonStreamProperty(pipeCall)
144+
}
145+
112146
/**
113147
* Gets a reference to a stream that may be the source of the given pipe call.
114148
* Uses type back-tracking to trace stream references in the data flow.
@@ -145,6 +179,6 @@ predicate hasErrorHandlerRegistered(PipeCall pipeCall) {
145179
from PipeCall pipeCall
146180
where
147181
not hasErrorHandlerRegistered(pipeCall) and
148-
not isPipeFollowedByNonStreamMethod(pipeCall)
182+
not isPipeFollowedByNonStreamAccess(pipeCall)
149183
select pipeCall,
150184
"Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped."

‎javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.expected

Copy file name to clipboardExpand all lines: javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.expected
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,5 @@
1111
| test.js:143:5:143:62 | stream. ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
1212
| test.js:171:5:171:50 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
1313
| test.js:175:5:175:39 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
14-
| test.js:179:17:179:40 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
1514
| test.js:183:17:183:40 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
1615
| test.js:193:5:193:32 | copyStr ... nation) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |

‎javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.js

Copy file name to clipboardExpand all lines: javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ function test() {
176176
}
177177
{ // Member access on a non-stream after pipe
178178
const notStream = getNotAStream();
179-
const val = notStream.pipe(writable).someMember; // $SPURIOUS:Alert
179+
const val = notStream.pipe(writable).someMember;
180180
}
181181
{ // Member access on a stream after pipe
182182
const notStream = getNotAStream();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.