Skip to content

Commit bedf354

Browse files
hahwulhahwul
authored
Merge pull request #1776 from owasp-noir/fan-meal
Improve Kotlin route extraction accuracy
2 parents 3775cd6 + 5c575d1 commit bedf354

7 files changed

Lines changed: 560 additions & 41 deletions

File tree

‎spec/unit_test/miniparser/http4k_extractor_ts_spec.cr‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
require "spec"
2+
require "../../../src/miniparsers/http4k_extractor_ts"
3+
4+
describe Noir::TreeSitterHttp4kExtractor do
5+
it "resolves constant and templated route paths" do
6+
source = <<-KT
7+
package com.example
8+
9+
import org.http4k.core.Method.GET
10+
import org.http4k.core.Response
11+
import org.http4k.core.Status.Companion.OK
12+
import org.http4k.routing.bind
13+
import org.http4k.routing.routes
14+
15+
object Paths {
16+
const val API = "/api"
17+
}
18+
19+
const val USERS = "/users"
20+
21+
val app = routes(
22+
Paths.API bind routes(
23+
(USERS + "/{id}") bind GET to { req -> Response(OK) },
24+
"/tenants/$tenantId/items" bind GET to { req -> Response(OK) }
25+
)
26+
)
27+
KT
28+
29+
constants = Noir::TreeSitterHttp4kExtractor.extract_string_constants(source)
30+
routes = Noir::TreeSitterHttp4kExtractor.extract_routes(source, constants)
31+
routes.map { |r| {r.verb, r.path} }.should eq([
32+
{"GET", "/api/users/{id}"},
33+
{"GET", "/api/tenants/{tenantId}/items"},
34+
])
35+
end
36+
37+
it "does not resolve route paths from project-wide bare constants" do
38+
source = <<-KT
39+
import org.http4k.core.Method.GET
40+
import org.http4k.core.Response
41+
import org.http4k.core.Status.Companion.OK
42+
import org.http4k.routing.bind
43+
import org.http4k.routing.routes
44+
45+
val app = routes(
46+
USERS bind GET to { req -> Response(OK) }
47+
)
48+
KT
49+
50+
routes = Noir::TreeSitterHttp4kExtractor.extract_routes(source, {
51+
"USERS" => "/wrong",
52+
})
53+
routes.should be_empty
54+
end
55+
56+
it "does not drop qualifiers when resolving route constants" do
57+
source = <<-KT
58+
import org.http4k.core.Method.GET
59+
import org.http4k.core.Response
60+
import org.http4k.core.Status.Companion.OK
61+
import org.http4k.routing.bind
62+
import org.http4k.routing.routes
63+
64+
val app = routes(
65+
Other.API bind GET to { req -> Response(OK) }
66+
)
67+
KT
68+
69+
routes = Noir::TreeSitterHttp4kExtractor.extract_routes(source, {
70+
"API" => "/wrong",
71+
})
72+
routes.should be_empty
73+
end
74+
end

‎spec/unit_test/miniparser/kotlin_ktor_route_extractor_ts_spec.cr‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,62 @@ describe Noir::TreeSitterKotlinKtorRouteExtractor do
8787
])
8888
end
8989

90+
it "resolves constant and templated route paths" do
91+
source = <<-KT
92+
package com.example
93+
94+
object Paths {
95+
const val API = "/api"
96+
}
97+
98+
const val USERS = "/users"
99+
100+
routing {
101+
route(Paths.API + "/v1") {
102+
get(USERS) { }
103+
get("/tenants/$tenantId/items") { }
104+
}
105+
}
106+
KT
107+
108+
constants = Noir::TreeSitterKotlinKtorRouteExtractor.extract_string_constants(source)
109+
routes = Noir::TreeSitterKotlinKtorRouteExtractor.extract_routes(source, constants)
110+
routes.map { |r| {r.verb, r.path} }.should eq([
111+
{"GET", "/api/v1/users"},
112+
{"GET", "/api/v1/tenants/{tenantId}/items"},
113+
])
114+
end
115+
116+
it "does not resolve route paths from project-wide bare constants" do
117+
source = <<-KT
118+
routing {
119+
get(USERS) { }
120+
route(API) {
121+
get("/nested") { }
122+
}
123+
}
124+
KT
125+
126+
routes = Noir::TreeSitterKotlinKtorRouteExtractor.extract_routes(source, {
127+
"USERS" => "/wrong-users",
128+
"API" => "/wrong-api",
129+
})
130+
routes.should be_empty
131+
end
132+
133+
it "does not drop qualifiers when resolving route constants" do
134+
source = <<-KT
135+
routing {
136+
get(Other.API) { }
137+
}
138+
KT
139+
140+
routes = Noir::TreeSitterKotlinKtorRouteExtractor.extract_routes(source, {
141+
"API" => "/wrong",
142+
})
143+
routes.should be_empty
144+
end
145+
90146
it "treats install(RoutingRoot) as a routing entry point" do
91147
source = <<-KT
92148
fun Application.module() {
@@ -183,6 +239,30 @@ describe Noir::TreeSitterKotlinKtorRouteExtractor do
183239
route.header_params.should eq(["X-API-Key", "Authorization"])
184240
end
185241

242+
it "captures common query, header, body, and form access variants" do
243+
source = <<-KT
244+
routing {
245+
post("/profile") {
246+
val query = call.request.queryParameters["preview"]
247+
val page = call.request.queryParameters.get("page")
248+
val id = call.parameters.get("id")
249+
val apiKey = call.request.headers.get("X-API-Key")
250+
val auth = call.request.header("Authorization")
251+
val form = call.receiveParameters()
252+
val email = form["email"]
253+
val phone = form.get("phone")
254+
val body = call.receiveText()
255+
}
256+
}
257+
KT
258+
259+
route = Noir::TreeSitterKotlinKtorRouteExtractor.extract_routes(source).first
260+
route.has_body?.should be_true
261+
route.query_params.should eq(["preview", "page", "id"])
262+
route.header_params.should eq(["X-API-Key", "Authorization"])
263+
route.form_params.should eq(["email", "phone"])
264+
end
265+
186266
it "ignores params on sibling routes" do
187267
source = <<-KT
188268
routing {

‎src/analyzer/analyzers/kotlin/http4k.cr‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,19 @@ module Analyzer::Kotlin
1010
def analyze
1111
include_callee = any_to_bool(@options["include_callee"]?) || any_to_bool(@options["ai_context"]?)
1212
file_list = all_files()
13+
string_constants = Hash(String, String).new
14+
file_list.each do |path|
15+
next unless File.exists?(path)
16+
next unless path.ends_with?(".#{KOTLIN_EXTENSION}")
17+
next if KotlinEngine.test_path?(path)
18+
19+
Noir::TreeSitterHttp4kExtractor.extract_string_constants(read_file_content(path)).each do |name, value|
20+
next unless fully_qualified_constant?(name)
21+
22+
string_constants[name] ||= value
23+
end
24+
end
25+
1326
file_list.each do |path|
1427
next unless File.exists?(path)
1528
next unless path.ends_with?(".#{KOTLIN_EXTENSION}")
@@ -18,7 +31,7 @@ module Analyzer::Kotlin
1831
content = read_file_content(path)
1932
next unless content.includes?(HTTP4K_MARKER)
2033

21-
Noir::TreeSitterHttp4kExtractor.extract_routes(content, include_callees: include_callee).each do |route|
34+
Noir::TreeSitterHttp4kExtractor.extract_routes(content, string_constants, include_callees: include_callee).each do |route|
2235
@result << build_endpoint(route, path)
2336
end
2437
end
@@ -27,6 +40,10 @@ module Analyzer::Kotlin
2740
@result
2841
end
2942

43+
private def fully_qualified_constant?(name : String) : Bool
44+
name.count('.') >= 2
45+
end
46+
3047
private def build_endpoint(route : Noir::TreeSitterHttp4kExtractor::Route, path : String) : Endpoint
3148
params = [] of Param
3249
route.query_params.each { |name| params << Param.new(name, "", "query") }

‎src/analyzer/analyzers/kotlin/ktor.cr‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ module Analyzer::Kotlin
99
def analyze
1010
include_callee = any_to_bool(@options["include_callee"]?) || any_to_bool(@options["ai_context"]?)
1111
file_list = all_files()
12+
string_constants = Hash(String, String).new
13+
file_list.each do |path|
14+
next unless File.exists?(path)
15+
next unless path.ends_with?(".#{KOTLIN_EXTENSION}")
16+
next if KotlinEngine.test_path?(path)
17+
18+
Noir::TreeSitterKotlinKtorRouteExtractor.extract_string_constants(read_file_content(path)).each do |name, value|
19+
next unless fully_qualified_constant?(name)
20+
21+
string_constants[name] ||= value
22+
end
23+
end
24+
1225
file_list.each do |path|
1326
next unless File.exists?(path)
1427
next unless path.ends_with?(".#{KOTLIN_EXTENSION}")
@@ -17,7 +30,7 @@ module Analyzer::Kotlin
1730
content = read_file_content(path)
1831
next unless potential_ktor_route_file?(content)
1932

20-
Noir::TreeSitterKotlinKtorRouteExtractor.extract_routes(content, include_callees: include_callee).each do |route|
33+
Noir::TreeSitterKotlinKtorRouteExtractor.extract_routes(content, string_constants, include_callees: include_callee).each do |route|
2134
@result << build_endpoint(route, path)
2235
end
2336
end
@@ -32,6 +45,10 @@ module Analyzer::Kotlin
3245
content.includes?("Route.")
3346
end
3447

48+
private def fully_qualified_constant?(name : String) : Bool
49+
name.count('.') >= 2
50+
end
51+
3552
private def build_endpoint(route : Noir::TreeSitterKotlinKtorRouteExtractor::Route, path : String) : Endpoint
3653
details = Details.new(PathInfo.new(path, route.line + 1))
3754
params = [] of Param
@@ -48,6 +65,8 @@ module Analyzer::Kotlin
4865

4966
if rt = route.receive_type
5067
params << Param.new("body", rt, "json")
68+
elsif route.has_body?
69+
params << Param.new("body", "", "json")
5170
end
5271

5372
route.query_params.each do |name|
@@ -59,6 +78,10 @@ module Analyzer::Kotlin
5978
params << Param.new(name, "", "header")
6079
end
6180

81+
route.form_params.each do |name|
82+
params << Param.new(name, "", "form")
83+
end
84+
6285
endpoint = Endpoint.new(route.path, route.verb, params, details)
6386

6487
# 1-hop callees out of the handler lambda body. The Route

0 commit comments

Comments
 (0)