Skip to content

Commit 0a67902

Browse files
authored
Merge pull request #20101 from mschwager/main
Fix #19294, Ruby NetHttpRequest improvements
2 parents 2b92b83 + 357964e commit 0a67902

File tree

4 files changed

+44
-5
lines changed

4 files changed

+44
-5
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
category: fix
3+
---
4+
* Made the following changes to `NetHttpRequest`
5+
* Adds `connectionNode`, like other Ruby HTTP clients
6+
* Makes `requestNode` and `connectionNode` public so subclasses can use them
7+
* Adds detection of `Net::HTTP.start`, a common way to make HTTP requests in Ruby

ruby/ql/lib/codeql/ruby/frameworks/http_clients/NetHttp.qll

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,38 +12,55 @@ private import codeql.ruby.DataFlow
1212
/**
1313
* A `Net::HTTP` call which initiates an HTTP request.
1414
* ```ruby
15+
* # one-off request
1516
* Net::HTTP.get("http://example.com/")
1617
* Net::HTTP.post("http://example.com/", "some_data")
1718
* req = Net::HTTP.new("example.com")
1819
* response = req.get("/")
20+
*
21+
* # connection re-use
22+
* Net::HTTP.start("http://example.com") do |http|
23+
* http.get("/")
24+
* end
1925
* ```
2026
*/
2127
class NetHttpRequest extends Http::Client::Request::Range instanceof DataFlow::CallNode {
2228
private DataFlow::CallNode request;
23-
private API::Node requestNode;
29+
API::Node requestNode;
30+
API::Node connectionNode;
2431
private boolean returnsResponseBody;
2532

2633
NetHttpRequest() {
2734
exists(string method |
2835
request = requestNode.asSource() and
29-
this = request
36+
this = request and
37+
requestNode = connectionNode.getReturn(method)
3038
|
3139
// Net::HTTP.get(...)
3240
method in ["get", "get_response"] and
33-
requestNode = API::getTopLevelMember("Net").getMember("HTTP").getReturn(method) and
41+
connectionNode = API::getTopLevelMember("Net").getMember("HTTP") and
3442
returnsResponseBody = true
3543
or
3644
// Net::HTTP.post(...).body
3745
method in ["post", "post_form"] and
38-
requestNode = API::getTopLevelMember("Net").getMember("HTTP").getReturn(method) and
46+
connectionNode = API::getTopLevelMember("Net").getMember("HTTP") and
3947
returnsResponseBody = false
4048
or
4149
// Net::HTTP.new(..).get(..).body
50+
// Net::HTTP.start(..) do |http| http.get(..) end
4251
method in [
4352
"get", "get2", "request_get", "head", "head2", "request_head", "delete", "put", "patch",
4453
"post", "post2", "request_post", "request"
4554
] and
46-
requestNode = API::getTopLevelMember("Net").getMember("HTTP").getInstance().getReturn(method) and
55+
connectionNode =
56+
[
57+
API::getTopLevelMember("Net").getMember("HTTP").getInstance(),
58+
API::getTopLevelMember("Net")
59+
.getMember("HTTP")
60+
.getMethod("start")
61+
.getBlock()
62+
.getParameter(0)
63+
] and
4764
returnsResponseBody = false
4865
)
4966
}

ruby/ql/test/library-tests/frameworks/http_clients/HttpClients.expected

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ httpRequests
4646
| NetHttp.rb:16:6:16:19 | call to patch |
4747
| NetHttp.rb:24:3:24:33 | call to get |
4848
| NetHttp.rb:29:1:29:32 | call to post |
49+
| NetHttp.rb:33:1:33:22 | call to request |
50+
| NetHttp.rb:36:3:36:15 | call to get |
4951
| OpenURI.rb:3:9:3:41 | call to open |
5052
| OpenURI.rb:6:9:6:34 | call to open |
5153
| OpenURI.rb:9:9:9:38 | call to open |
@@ -123,6 +125,8 @@ getFramework
123125
| NetHttp.rb:16:6:16:19 | call to patch | Net::HTTP |
124126
| NetHttp.rb:24:3:24:33 | call to get | Net::HTTP |
125127
| NetHttp.rb:29:1:29:32 | call to post | Net::HTTP |
128+
| NetHttp.rb:33:1:33:22 | call to request | Net::HTTP |
129+
| NetHttp.rb:36:3:36:15 | call to get | Net::HTTP |
126130
| OpenURI.rb:3:9:3:41 | call to open | OpenURI |
127131
| OpenURI.rb:6:9:6:34 | call to open | OpenURI |
128132
| OpenURI.rb:9:9:9:38 | call to open | OpenURI |
@@ -292,6 +296,9 @@ getAUrlPart
292296
| NetHttp.rb:24:3:24:33 | call to get | NetHttp.rb:24:17:24:22 | domain |
293297
| NetHttp.rb:24:3:24:33 | call to get | NetHttp.rb:24:29:24:32 | path |
294298
| NetHttp.rb:29:1:29:32 | call to post | NetHttp.rb:29:16:29:18 | uri |
299+
| NetHttp.rb:33:1:33:22 | call to request | NetHttp.rb:31:22:31:42 | "https://example.com" |
300+
| NetHttp.rb:33:1:33:22 | call to request | NetHttp.rb:33:14:33:21 | root_get |
301+
| NetHttp.rb:36:3:36:15 | call to get | NetHttp.rb:36:12:36:14 | "/" |
295302
| OpenURI.rb:3:9:3:41 | call to open | OpenURI.rb:3:21:3:40 | "http://example.com" |
296303
| OpenURI.rb:6:9:6:34 | call to open | OpenURI.rb:6:14:6:33 | "http://example.com" |
297304
| OpenURI.rb:9:9:9:38 | call to open | OpenURI.rb:9:18:9:37 | "http://example.com" |

ruby/ql/test/library-tests/frameworks/http_clients/NetHttp.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,11 @@ def get(domain, path)
2727
get("example.com", "/").body
2828

2929
Net::HTTP.post(uri, "some_body") # note: response body not accessed
30+
31+
http = Net::HTTP.new("https://example.com")
32+
root_get = Net::HTTP::Get.new("/")
33+
http.request(root_get)
34+
35+
Net::HTTP.start("https://example.com") do |http|
36+
http.get("/")
37+
end

0 commit comments

Comments
 (0)