Skip to content

Commit 226d322

Browse files
vicansovicanso
committed
refactor: adjust proxy handle function
1 parent be88a90 commit 226d322

File tree

10 files changed

+64
-52
lines changed

10 files changed

+64
-52
lines changed

Diff for: ‎benches/bench.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,8 @@ fn bench_location_filter(c: &mut Criterion) {
139139
)
140140
.unwrap();
141141
b.iter(|| {
142-
lo.matched("", "/api/users/me");
143-
lo.matched("", "/rest");
142+
lo.match_host_path("", "/api/users/me");
143+
lo.match_host_path("", "/rest");
144144
});
145145
});
146146

@@ -155,8 +155,8 @@ fn bench_location_filter(c: &mut Criterion) {
155155
)
156156
.unwrap();
157157
b.iter(|| {
158-
lo.matched("", "/rest/api/users/me");
159-
lo.matched("", "/rest");
158+
lo.match_host_path("", "/rest/api/users/me");
159+
lo.match_host_path("", "/rest");
160160
});
161161
});
162162
group.bench_function("equal", |b| {
@@ -170,8 +170,8 @@ fn bench_location_filter(c: &mut Criterion) {
170170
)
171171
.unwrap();
172172
b.iter(|| {
173-
lo.matched("", "/api/users/me");
174-
lo.matched("", "/api");
173+
lo.match_host_path("", "/api/users/me");
174+
lo.match_host_path("", "/api");
175175
});
176176
});
177177

Diff for: ‎error.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
>
5959
</div>
6060
</header>
61-
<p class="pingap-error">{{error_ype}}</p>
61+
<p class="pingap-error">{{error_type}}</p>
6262
<p class="pingap-error">{{content}}</p>
6363
</body>
6464
</html>

Diff for: ‎src/plugin/cache.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ impl Plugin for Cache {
390390
if method == METHOD_PURGE.to_owned() {
391391
let found = match self
392392
.purge_ip_rules
393-
.matched(&util::get_client_ip(session))
393+
.is_match(&util::get_client_ip(session))
394394
{
395395
Ok(matched) => matched,
396396
Err(e) => {

Diff for: ‎src/plugin/combined_auth.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ impl CombinedAuth {
183183
// Uses X-Forwarded-For header for IP detection behind proxies
184184
if let Some(ip_rules) = &auth_param.ip_rules {
185185
let ip = util::get_client_ip(session);
186-
if !ip_rules.matched(&ip).unwrap_or_default() {
186+
if !ip_rules.is_match(&ip).unwrap_or_default() {
187187
return Err(Error::Invalid {
188188
category: category.to_string(),
189189
message: "ip is invalid".to_string(),

Diff for: ‎src/plugin/ip_restriction.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ impl Plugin for IpRestriction {
147147

148148
// Check if IP matches any configured rules
149149
// Returns error if IP is malformed
150-
let found = match self.ip_rules.matched(&ip) {
150+
let found = match self.ip_rules.is_match(&ip) {
151151
Ok(matched) => matched,
152152
Err(e) => {
153153
return Ok(Some(HttpResponse::bad_request(

Diff for: ‎src/proxy/dynamic_certificate.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -292,12 +292,15 @@ impl pingora::listeners::TlsAccept for GlobalCertificate {
292292
// 5. Handle special case for CA certificates (self-signed)
293293
// 6. Apply certificate, private key, and chain to SSL context
294294

295-
// TODO add more debug log
296-
debug!(category = LOG_CATEGORY, ssl = format!("{ssl:?}"));
297295
let sni = ssl
298296
.servername(NameType::HOST_NAME)
299297
.unwrap_or(DEFAULT_SERVER_NAME);
300-
debug!(category = LOG_CATEGORY, server_name = sni);
298+
// TODO add more debug log
299+
debug!(
300+
category = LOG_CATEGORY,
301+
ssl = format!("{ssl:?}"),
302+
server_name = sni
303+
);
301304

302305
let mut dynamic_certificate = None;
303306
let certs = DYNAMIC_CERTIFICATE_MAP.load();

Diff for: ‎src/proxy/location.rs

+18-18
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ impl Location {
431431
/// - bool: Whether the request matched both path and host rules
432432
/// - Option<Vec<(String, String)>>: Any captured variables from regex host matching
433433
#[inline]
434-
pub fn matched(
434+
pub fn match_host_path(
435435
&self,
436436
host: &str,
437437
path: &str,
@@ -787,8 +787,8 @@ mod tests {
787787
},
788788
)
789789
.unwrap();
790-
assert_eq!(true, lo.matched("pingap", "/api").0);
791-
assert_eq!(true, lo.matched("", "").0);
790+
assert_eq!(true, lo.match_host_path("pingap", "/api").0);
791+
assert_eq!(true, lo.match_host_path("", "").0);
792792

793793
// host
794794
let lo = Location::new(
@@ -800,9 +800,9 @@ mod tests {
800800
},
801801
)
802802
.unwrap();
803-
assert_eq!(true, lo.matched("pingap", "/api").0);
804-
assert_eq!(true, lo.matched("pingap", "").0);
805-
assert_eq!(false, lo.matched("", "/api").0);
803+
assert_eq!(true, lo.match_host_path("pingap", "/api").0);
804+
assert_eq!(true, lo.match_host_path("pingap", "").0);
805+
assert_eq!(false, lo.match_host_path("", "/api").0);
806806

807807
// regex
808808
let lo = Location::new(
@@ -814,9 +814,9 @@ mod tests {
814814
},
815815
)
816816
.unwrap();
817-
assert_eq!(true, lo.matched("", "/api/users").0);
818-
assert_eq!(true, lo.matched("", "/users").0);
819-
assert_eq!(false, lo.matched("", "/api").0);
817+
assert_eq!(true, lo.match_host_path("", "/api/users").0);
818+
assert_eq!(true, lo.match_host_path("", "/users").0);
819+
assert_eq!(false, lo.match_host_path("", "/api").0);
820820

821821
// regex ^/api
822822
let lo = Location::new(
@@ -828,9 +828,9 @@ mod tests {
828828
},
829829
)
830830
.unwrap();
831-
assert_eq!(true, lo.matched("", "/api/users").0);
832-
assert_eq!(false, lo.matched("", "/users").0);
833-
assert_eq!(true, lo.matched("", "/api").0);
831+
assert_eq!(true, lo.match_host_path("", "/api/users").0);
832+
assert_eq!(false, lo.match_host_path("", "/users").0);
833+
assert_eq!(true, lo.match_host_path("", "/api").0);
834834

835835
// prefix
836836
let lo = Location::new(
@@ -842,9 +842,9 @@ mod tests {
842842
},
843843
)
844844
.unwrap();
845-
assert_eq!(true, lo.matched("", "/api/users").0);
846-
assert_eq!(false, lo.matched("", "/users").0);
847-
assert_eq!(true, lo.matched("", "/api").0);
845+
assert_eq!(true, lo.match_host_path("", "/api/users").0);
846+
assert_eq!(false, lo.match_host_path("", "/users").0);
847+
assert_eq!(true, lo.match_host_path("", "/api").0);
848848

849849
// equal
850850
let lo = Location::new(
@@ -856,9 +856,9 @@ mod tests {
856856
},
857857
)
858858
.unwrap();
859-
assert_eq!(false, lo.matched("", "/api/users").0);
860-
assert_eq!(false, lo.matched("", "/users").0);
861-
assert_eq!(true, lo.matched("", "/api").0);
859+
assert_eq!(false, lo.match_host_path("", "/api/users").0);
860+
assert_eq!(false, lo.match_host_path("", "/users").0);
861+
assert_eq!(true, lo.match_host_path("", "/api").0);
862862
}
863863

864864
#[test]

Diff for: ‎src/proxy/server.rs

+27-19
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ fn get_digest_detail(digest: &Digest) -> DigestDetail {
492492
connection_reused,
493493
tcp_established,
494494
connection_time,
495-
tls_established: get_established(digest.timing_digest.get(1)),
495+
tls_established: get_established(digest.timing_digest.last()),
496496
tls_version: Some(ssl_digest.version.to_string()),
497497
tls_cipher: Some(ssl_digest.cipher.to_string()),
498498
}
@@ -609,7 +609,7 @@ impl ProxyHttp for Server {
609609
let Some(location) = get_location(name) else {
610610
continue;
611611
};
612-
let (matched, variables) = location.matched(host, path);
612+
let (matched, variables) = location.match_host_path(host, path);
613613
if matched {
614614
ctx.location = Some(location);
615615
if let Some(variables) = variables {
@@ -633,6 +633,17 @@ impl ProxyHttp for Server {
633633
.validate_content_length(header)
634634
.map_err(|e| util::new_internal_error(413, e.to_string()))?;
635635

636+
// add processing, if processing is max than limit,
637+
// it will return error with 429 status code
638+
match location.add_processing() {
639+
Ok((accepted, processing)) => {
640+
ctx.location_accepted = accepted;
641+
ctx.location_processing = processing;
642+
},
643+
Err(e) => {
644+
return Err(util::new_internal_error(429, e.to_string()));
645+
},
646+
};
636647
if location.support_grpc_web() {
637648
// Initialize grpc web module for this request
638649
let grpc_web = session
@@ -648,15 +659,6 @@ impl ProxyHttp for Server {
648659
grpc_web.init();
649660
}
650661

651-
match location.add_processing() {
652-
Ok((accepted, processing)) => {
653-
ctx.location_accepted = accepted;
654-
ctx.location_processing = processing;
655-
},
656-
Err(e) => {
657-
return Err(util::new_internal_error(429, e.to_string()));
658-
},
659-
};
660662
let _ = location
661663
.clone()
662664
.handle_request_plugin(PluginStep::EarlyRequest, session, ctx)
@@ -706,10 +708,15 @@ impl ProxyHttp for Server {
706708
&& self.prometheus.is_some()
707709
&& header.uri.path() == self.prometheus_metrics
708710
{
709-
let body =
710-
self.prometheus.as_ref().unwrap().metrics().map_err(|e| {
711-
util::new_internal_error(500, e.to_string())
712-
})?;
711+
let body = self
712+
.prometheus
713+
.as_ref()
714+
.ok_or(util::new_internal_error(
715+
500,
716+
"get prometheus fail".to_string(),
717+
))?
718+
.metrics()
719+
.map_err(|e| util::new_internal_error(500, e.to_string()))?;
713720
HttpResponse::text(body.into()).send(session).await?;
714721
return Ok(true);
715722
}
@@ -833,9 +840,9 @@ impl ProxyHttp for Server {
833840
{
834841
debug!(category = LOG_CATEGORY, "--> connected to upstream");
835842
defer!(debug!(category = LOG_CATEGORY, "<-- connected to upstream"););
836-
if !reused {
837-
if let Some(digest) = digest {
838-
let detail = get_digest_detail(digest);
843+
if let Some(digest) = digest {
844+
let detail = get_digest_detail(digest);
845+
if !reused {
839846
let upstream_connect_time =
840847
ctx.upstream_connect_time.unwrap_or_default();
841848
if upstream_connect_time > 0
@@ -849,6 +856,7 @@ impl ProxyHttp for Server {
849856
Some(detail.tls_established - detail.tcp_established);
850857
}
851858
}
859+
ctx.upstream_connection_time = Some(detail.connection_time);
852860
}
853861

854862
ctx.upstream_reused = reused;
@@ -1192,7 +1200,7 @@ impl ProxyHttp for Server {
11921200
.error_template
11931201
.replace("{{version}}", util::get_pkg_version())
11941202
.replace("{{content}}", &e.to_string())
1195-
.replace("{{error_ype}}", error_type);
1203+
.replace("{{error_type}}", error_type);
11961204
let buf = Bytes::from(content);
11971205
ctx.status = Some(
11981206
StatusCode::from_u16(code)

Diff for: ‎src/proxy/server_conf.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,11 @@ pub struct ServerConf {
7272
pub tcp_fastopen: Option<usize>,
7373

7474
// Whether to use globally configured TLS certificates
75-
// False means server-specific certificates will be used
75+
// False means the server is using http protocol
7676
pub global_certificates: bool,
7777

7878
// Whether HTTP/2 protocol support is enabled for this server
79+
// False means the server is using h2c protocol
7980
pub enabled_h2: bool,
8081

8182
// Endpoint path for exposing Prometheus metrics

Diff for: ‎src/util/ip.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ impl IpRules {
5555
/// - Ok(true) if IP matches either an individual IP or falls within a network range
5656
/// - Ok(false) if no match is found
5757
/// - Err if the IP address string cannot be parsed
58-
pub fn matched(&self, ip: &String) -> Result<bool, AddrParseError> {
58+
pub fn is_match(&self, ip: &String) -> Result<bool, AddrParseError> {
5959
let found = if self.ip_list.contains(ip) {
6060
// First check for exact match in individual IP list
6161
true

0 commit comments

Comments
 (0)