Skip to content

Commit f8fc583

Browse files
committed
Python: client request: getUrl => getAUrlPart
I think `getUrl` is a bit too misleading, since from the name, I would only ever expect ONE result for one request being made. `getAUrlPart` captures that there could be multiple results, and that they might not constitute a whole URl. Which is the same naming I used when I tried to model this a long time ago https://github.com/github/codeql/blob/a80860cdc6b06b363b0d0919600ab383a470b449/python/ql/lib/semmle/python/web/Http.qll#L102-L111
1 parent 6f81685 commit f8fc583

File tree

7 files changed

+54
-54
lines changed

7 files changed

+54
-54
lines changed

python/ql/lib/semmle/python/Concepts.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -829,7 +829,7 @@ module HTTP {
829829
* Gets a node that contributes to the URL of the request.
830830
* Depending on the framework, a request may have multiple nodes which contribute to the URL.
831831
*/
832-
DataFlow::Node getUrl() { result = super.getUrl() }
832+
DataFlow::Node getAUrlPart() { result = super.getAUrlPart() }
833833

834834
/** Gets a string that identifies the framework used for this request. */
835835
string getFramework() { result = super.getFramework() }
@@ -864,7 +864,7 @@ module HTTP {
864864
* Gets a node that contributes to the URL of the request.
865865
* Depending on the framework, a request may have multiple nodes which contribute to the URL.
866866
*/
867-
abstract DataFlow::Node getUrl();
867+
abstract DataFlow::Node getAUrlPart();
868868

869869
/** Gets a string that identifies the framework used for this request. */
870870
abstract string getFramework();
@@ -888,7 +888,7 @@ module HTTP {
888888
private class HttpClientRequestAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
889889
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
890890
exists(Request req |
891-
nodeFrom = req.getUrl() and
891+
nodeFrom = req.getAUrlPart() and
892892
nodeTo = req.getResponse()
893893
)
894894
}

python/ql/lib/semmle/python/frameworks/Requests.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ private module Requests {
4343
)
4444
}
4545

46-
override DataFlow::Node getUrl() {
46+
override DataFlow::Node getAUrlPart() {
4747
result = this.getArgByName("url")
4848
or
4949
not methodName = "request" and

python/ql/lib/semmle/python/frameworks/Stdlib.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2179,7 +2179,7 @@ private module StdlibPrivate {
21792179
this.getObject().getALocalSource()
21802180
}
21812181

2182-
override DataFlow::Node getUrl() {
2182+
override DataFlow::Node getAUrlPart() {
21832183
result in [this.getArg(1), this.getArgByName("url")]
21842184
or
21852185
this.getObject() = instance(result)

python/ql/test/experimental/meta/ConceptsTest.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -480,17 +480,17 @@ class HttpClientRequestTest extends InlineExpectationsTest {
480480
HttpClientRequestTest() { this = "HttpClientRequestTest" }
481481

482482
override string getARelevantTag() {
483-
result in ["clientRequestUrl", "clientRequestCertValidationDisabled"]
483+
result in ["clientRequestUrlPart", "clientRequestCertValidationDisabled"]
484484
}
485485

486486
override predicate hasActualResult(Location location, string element, string tag, string value) {
487487
exists(location.getFile().getRelativePath()) and
488488
exists(HTTP::Client::Request req, DataFlow::Node url |
489-
url = req.getUrl() and
489+
url = req.getAUrlPart() and
490490
location = url.getLocation() and
491491
element = url.toString() and
492492
value = prettyNodeForInlineTest(url) and
493-
tag = "clientRequestUrl"
493+
tag = "clientRequestUrlPart"
494494
)
495495
or
496496
exists(location.getFile().getRelativePath()) and

python/ql/test/library-tests/frameworks/requests/taint_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ def test_taint(): # $ requestHandler
1111

1212
# response from a request to a user-controlled URL should be considered
1313
# user-controlled as well.
14-
resp = requests.get(url) # $ clientRequestUrl=url
14+
resp = requests.get(url) # $ clientRequestUrlPart=url
1515

1616
requests.Response
1717
requests.models.Response
@@ -50,5 +50,5 @@ def test_taint(): # $ requestHandler
5050
# flow source, since this could lead to FPs.
5151
# TODO: investigate whether we should consider this a remote flow source.
5252
trusted_url = "https://internal-api-that-i-trust.com"
53-
resp = requests.get(trusted_url) # $ clientRequestUrl=trusted_url
53+
resp = requests.get(trusted_url) # $ clientRequestUrlPart=trusted_url
5454
ensure__not_tainted(resp)
Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,50 @@
11
import requests
22

3-
resp = requests.get("url") # $ clientRequestUrl="url"
4-
resp = requests.get(url="url") # $ clientRequestUrl="url"
3+
resp = requests.get("url") # $ clientRequestUrlPart="url"
4+
resp = requests.get(url="url") # $ clientRequestUrlPart="url"
55

6-
resp = requests.request("GET", "url") # $ clientRequestUrl="url"
6+
resp = requests.request("GET", "url") # $ clientRequestUrlPart="url"
77

88
with requests.Session() as session:
9-
resp = session.get("url") # $ clientRequestUrl="url"
10-
resp = session.request(method="GET", url="url") # $ clientRequestUrl="url"
9+
resp = session.get("url") # $ clientRequestUrlPart="url"
10+
resp = session.request(method="GET", url="url") # $ clientRequestUrlPart="url"
1111

1212
s = requests.Session()
13-
resp = s.get("url") # $ clientRequestUrl="url"
13+
resp = s.get("url") # $ clientRequestUrlPart="url"
1414

1515
s = requests.session()
16-
resp = s.get("url") # $ clientRequestUrl="url"
16+
resp = s.get("url") # $ clientRequestUrlPart="url"
1717

1818
# test full import path for Session
1919
with requests.sessions.Session() as session:
20-
resp = session.get("url") # $ clientRequestUrl="url"
20+
resp = session.get("url") # $ clientRequestUrlPart="url"
2121

2222
# Low level access
23-
req = requests.Request("GET", "url") # $ MISSING: clientRequestUrl="url"
23+
req = requests.Request("GET", "url") # $ MISSING: clientRequestUrlPart="url"
2424
resp = s.send(req.prepare())
2525

2626
# other methods than GET
27-
resp = requests.post("url") # $ clientRequestUrl="url"
28-
resp = requests.patch("url") # $ clientRequestUrl="url"
29-
resp = requests.options("url") # $ clientRequestUrl="url"
27+
resp = requests.post("url") # $ clientRequestUrlPart="url"
28+
resp = requests.patch("url") # $ clientRequestUrlPart="url"
29+
resp = requests.options("url") # $ clientRequestUrlPart="url"
3030

3131
# ==============================================================================
3232
# Disabling certificate validation
3333
# ==============================================================================
3434

35-
resp = requests.get("url", verify=False) # $ clientRequestUrl="url" clientRequestCertValidationDisabled
35+
resp = requests.get("url", verify=False) # $ clientRequestUrlPart="url" clientRequestCertValidationDisabled
3636

3737
def make_get(verify_arg):
38-
resp = requests.get("url", verify=verify_arg) # $ clientRequestUrl="url" clientRequestCertValidationDisabled
38+
resp = requests.get("url", verify=verify_arg) # $ clientRequestUrlPart="url" clientRequestCertValidationDisabled
3939

4040
make_get(False)
4141

4242

4343
with requests.Session() as session:
4444
# see https://github.com/psf/requests/blob/39d0fdd9096f7dceccbc8f82e1eda7dd64717a8e/requests/sessions.py#L621
4545
session.verify = False
46-
resp = session.get("url") # $ clientRequestUrl="url" MISSING: clientRequestCertValidationDisabled
47-
resp = session.get("url", verify=True) # $ clientRequestUrl="url"
46+
resp = session.get("url") # $ clientRequestUrlPart="url" MISSING: clientRequestCertValidationDisabled
47+
resp = session.get("url", verify=True) # $ clientRequestUrlPart="url"
4848

49-
req = requests.Request("GET", "url") # $ MISSING: clientRequestUrl="url"
49+
req = requests.Request("GET", "url") # $ MISSING: clientRequestUrlPart="url"
5050
resp = session.send(req.prepare()) # $ MISSING: clientRequestCertValidationDisabled

python/ql/test/library-tests/frameworks/stdlib/http_client.py

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,35 +11,35 @@
1111

1212

1313
# NOTE: the URL may be relative to host, or may be full URL.
14-
conn = HTTPConnection("example.com") # $ clientRequestUrl="example.com"
15-
conn.request("GET", "/") # $ clientRequestUrl="/"
14+
conn = HTTPConnection("example.com") # $ clientRequestUrlPart="example.com"
15+
conn.request("GET", "/") # $ clientRequestUrlPart="/"
1616
url = "http://example.com/"
17-
conn.request("GET", url) # $ clientRequestUrl=url
17+
conn.request("GET", url) # $ clientRequestUrlPart=url
1818

1919
# kwargs
20-
conn = HTTPConnection(host="example.com") # $ clientRequestUrl="example.com"
21-
conn.request(method="GET", url="/") # $ clientRequestUrl="/"
20+
conn = HTTPConnection(host="example.com") # $ clientRequestUrlPart="example.com"
21+
conn.request(method="GET", url="/") # $ clientRequestUrlPart="/"
2222

2323
# using internal method... you shouldn't but you can
24-
conn._send_request("GET", "url", body=None, headers={}, encode_chunked=False) # $ clientRequestUrl="url"
24+
conn._send_request("GET", "url", body=None, headers={}, encode_chunked=False) # $ clientRequestUrlPart="url"
2525

2626
# low level sending of request
27-
conn.putrequest("GET", "url") # $ clientRequestUrl="url"
27+
conn.putrequest("GET", "url") # $ clientRequestUrlPart="url"
2828
conn.putheader("X-Foo", "value")
2929
conn.endheaders(message_body=None)
3030

3131
# HTTPS
32-
conn = HTTPSConnection("host") # $ clientRequestUrl="host"
33-
conn.request("GET", "url") # $ clientRequestUrl="url"
32+
conn = HTTPSConnection("host") # $ clientRequestUrlPart="host"
33+
conn.request("GET", "url") # $ clientRequestUrlPart="url"
3434

3535
# six aliases
3636
import six
3737

38-
conn = six.moves.http_client.HTTPConnection("host") # $ clientRequestUrl="host"
39-
conn.request("GET", "url") # $ clientRequestUrl="url"
38+
conn = six.moves.http_client.HTTPConnection("host") # $ clientRequestUrlPart="host"
39+
conn.request("GET", "url") # $ clientRequestUrlPart="url"
4040

41-
conn = six.moves.http_client.HTTPSConnection("host") # $ clientRequestUrl="host"
42-
conn.request("GET", "url") # $ clientRequestUrl="url"
41+
conn = six.moves.http_client.HTTPSConnection("host") # $ clientRequestUrlPart="host"
42+
conn.request("GET", "url") # $ clientRequestUrlPart="url"
4343

4444
# ==============================================================================
4545
# Certificate validation disabled
@@ -50,34 +50,34 @@
5050
assert context.check_hostname == True
5151
assert context.verify_mode == ssl.CERT_REQUIRED
5252

53-
conn = HTTPSConnection("host", context=context) # $ clientRequestUrl="host"
54-
conn.request("GET", "url") # $ clientRequestUrl="url"
53+
conn = HTTPSConnection("host", context=context) # $ clientRequestUrlPart="host"
54+
conn.request("GET", "url") # $ clientRequestUrlPart="url"
5555

5656
# `_create_default_https_context` is currently just an alias for `create_default_context`
5757
# which creates a context for SERVER_AUTH purpose.
5858
context = ssl.create_default_context()
5959
assert context.check_hostname == True
6060
assert context.verify_mode == ssl.CERT_REQUIRED
6161

62-
conn = HTTPSConnection("host", context=context) # $ clientRequestUrl="host"
63-
conn.request("GET", "url") # $ clientRequestUrl="url"
62+
conn = HTTPSConnection("host", context=context) # $ clientRequestUrlPart="host"
63+
conn.request("GET", "url") # $ clientRequestUrlPart="url"
6464

6565
# however, if you supply your own SSLContext, you need to set it manually
6666
context = ssl.SSLContext()
6767
assert context.check_hostname == False
6868
assert context.verify_mode == ssl.CERT_NONE
6969

70-
conn = HTTPSConnection("host", context=context) # $ clientRequestUrl="host"
71-
conn.request("GET", "url") # $ clientRequestUrl="url" MISSING: clientRequestCertValidationDisabled
70+
conn = HTTPSConnection("host", context=context) # $ clientRequestUrlPart="host"
71+
conn.request("GET", "url") # $ clientRequestUrlPart="url" MISSING: clientRequestCertValidationDisabled
7272

7373
# and if you misunderstood whether to use server/client in the purpose, you will also
7474
# get a context without hostname verification.
7575
context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH)
7676
assert context.check_hostname == False
7777
assert context.verify_mode == ssl.CERT_NONE
7878

79-
conn = HTTPSConnection("host", context=context) # $ clientRequestUrl="host"
80-
conn.request("GET", "url") # $ clientRequestUrl="url" MISSING: clientRequestCertValidationDisabled
79+
conn = HTTPSConnection("host", context=context) # $ clientRequestUrlPart="host"
80+
conn.request("GET", "url") # $ clientRequestUrlPart="url" MISSING: clientRequestCertValidationDisabled
8181

8282
# NOTICE that current documentation says
8383
#
@@ -90,17 +90,17 @@
9090
context.check_hostname = True
9191
assert context.verify_mode == ssl.CERT_REQUIRED
9292

93-
conn = HTTPSConnection("host", context=context) # $ clientRequestUrl="host"
94-
conn.request("GET", "url") # $ clientRequestUrl="url"
93+
conn = HTTPSConnection("host", context=context) # $ clientRequestUrlPart="host"
94+
conn.request("GET", "url") # $ clientRequestUrlPart="url"
9595

9696
# only setting verify_mode is not enough, since check_hostname is not enabled
9797

9898
context = ssl.SSLContext()
9999
context.verify_mode = ssl.CERT_REQUIRED
100100
assert context.check_hostname == False
101101

102-
conn = HTTPSConnection("host", context=context) # $ clientRequestUrl="host"
103-
conn.request("GET", "url") # $ clientRequestUrl="url" MISSING: clientRequestCertValidationDisabled
102+
conn = HTTPSConnection("host", context=context) # $ clientRequestUrlPart="host"
103+
conn.request("GET", "url") # $ clientRequestUrlPart="url" MISSING: clientRequestCertValidationDisabled
104104

105105
# ==============================================================================
106106
# taint test
@@ -112,8 +112,8 @@ def taint_test():
112112
host = request.args['host']
113113
url = request.args['url']
114114

115-
conn = HTTPConnection(host) # $ clientRequestUrl=host
116-
conn.request("GET", url) # $ clientRequestUrl=url
115+
conn = HTTPConnection(host) # $ clientRequestUrlPart=host
116+
conn.request("GET", url) # $ clientRequestUrlPart=url
117117

118118
resp = conn.getresponse()
119119

0 commit comments

Comments
 (0)