simplify uri path parsing

Signed-off-by: Dan Bond <[email protected]>

Author: loshz

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "metrics_server"
-version = "0.14.0"
+version = "0.15.0"
 authors = ["Dan Bond <[email protected]>"]
 edition = "2021"
 rust-version = "1.63"

README.md

@@ -16,13 +16,13 @@ This crate provides a thread safe, minimalstic HTTP/S server used to buffer metr
 Include the lib in your `Cargo.toml` dependencies:
 ```toml
 [dependencies]
-metrics_server = "0.14"
+metrics_server = "0.15"
 ```
 
 To enable TLS support, pass the optional feature flag:
 ```toml
 [dependencies]
-metrics_server = { version = "0.14", features = ["tls"] }
+metrics_server = { version = "0.15", features = ["tls"] }
 ```
 
 ### HTTP

src/server.rs

@@ -4,7 +4,7 @@ use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
 use std::thread;
 
-use http::Uri;
+use http::uri::PathAndQuery;
 use log::{debug, error};
 use time::{format_description, OffsetDateTime};
 use tiny_http::{ConfigListenAddr, Method, Response, Server};
@@ -127,16 +127,17 @@ impl MetricsServer {
     ///
     /// The server will only respond synchronously as it blocks until receiving new requests.
     /// Suqsequent calls to this method will return a no-op and not affect the underlying server.
-    pub fn serve_uri(&mut self, uri: String) {
+    pub fn serve_uri(&mut self, path: String) {
         // Check if we already have a thread running.
         if let Some(thread) = &self.thread {
             if !thread.is_finished() {
+                debug!("metrics server already running, continuing");
                 return;
             }
         }
 
-        // Ensure URI is valid.
-        let u = parse_uri(uri);
+        // Ensure path is valid.
+        let path = parse_path(&path);
 
         // Invoking clone on Arc produces a new Arc instance, which points to the
         // same allocation on the heap as the source Arc, while increasing a reference count.
@@ -149,11 +150,12 @@ impl MetricsServer {
                 for req in s.server.incoming_requests() {
                     // Check to see if we should stop handling requests.
                     if s.stop.load(Ordering::Relaxed) {
-                        break;
+                        debug!("metrics server stopping");
+                        return;
                     }
 
-                    // Only serve the specified uri path.
-                    if req.url().to_lowercase() != u {
+                    // Only serve the specified URI path.
+                    if req.url() != path {
                         let res = Response::empty(404);
                         respond(req, res);
                         continue;
@@ -198,16 +200,18 @@ impl MetricsServer {
     }
 }
 
-// Validate the provided URI or return the default /metrics on error.
-fn parse_uri(mut uri: String) -> String {
-    if !uri.starts_with('/') {
-        uri.insert(0, '/');
-    }
-
-    match Uri::from_str(&uri) {
-        Ok(u) => u.path().to_lowercase(),
+// Validate the provided URL path, or return the default path on error.
+fn parse_path(uri: &str) -> String {
+    match PathAndQuery::from_str(uri) {
+        Ok(pq) => {
+            let mut path = pq.path().to_lowercase();
+            if !path.starts_with('/') {
+                path.insert(0, '/');
+            }
+            path
+        }
         Err(_) => {
-            error!("invalid uri, defaulting to {}", DEFAULT_METRICS_PATH);
+            error!("invalid uri, defaulting to {DEFAULT_METRICS_PATH}");
             DEFAULT_METRICS_PATH.to_string()
         }
     }
@@ -242,21 +246,19 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_parse_uri() {
+    fn test_parse_path() {
         let expected_default = DEFAULT_METRICS_PATH.to_string();
-        let expected_valid = "/v1/metrics".to_string();
+        let expected_valid = "/debug/metrics".to_string();
 
         // Invalid.
-        assert_eq!(parse_uri("Hello, World!".to_string()), expected_default);
-        // No slash prefix.
-        assert_eq!(parse_uri("metrics".to_string()), expected_default);
-        // Leading slash prefix.
-        assert_eq!(parse_uri("/metrics".to_string()), expected_default);
+        assert_eq!(parse_path("Hello, World!"), expected_default);
         // Whitespace.
-        assert_eq!(parse_uri(" metr ics  ".to_string()), expected_default);
-        // Uppercase.
-        assert_eq!(parse_uri("METRICS".to_string()), expected_default);
+        assert_eq!(parse_path(" metr ics  "), expected_default);
+        // Non-ASCII.
+        assert_eq!(parse_path("mëtrîcs"), expected_default);
         // Valid.
-        assert_eq!(parse_uri("/v1/metrics".to_string()), expected_valid);
+        assert_eq!(parse_path("/debug/metrics"), expected_valid);
+        assert_eq!(parse_path("debug/metrics"), expected_valid);
+        assert_eq!(parse_path("DEBUG/METRICS"), expected_valid);
     }
 }

tests/server.rs

@@ -86,9 +86,9 @@ fn test_http_server_serve() {
     let res = reqwest::blocking::get("http://localhost:8001/metricsssss").unwrap();
     assert_eq!(404, res.status());
 
-    // Assert calls to uppercase URLs returns 200.
+    // Assert calls to uppercase URLs returns 404.
     let res = reqwest::blocking::get("http://localhost:8001/METRICS").unwrap();
-    assert_eq!(200, res.status());
+    assert_eq!(404, res.status());
 
     // Assert non GET requests to /metrics endpoint returns 405.
     let client = reqwest::blocking::Client::new();