Allow rpm test to run in non-tls mode #31

kornelski · 2025-09-30T21:43:41Z

having both no- and disable- is inconsistent. no- is more typical for args, so maybe rename the other arg to no-aim-scores and add disable-aim-scores as an alias for backwards compatibility?

kornelski · 2025-09-30T21:46:11Z

Please use tls: true.

--no-tls makes sense as an arg, but internally working with negated names is adding cognitive overhead.

kornelski · 2025-09-30T21:48:33Z

Reporting a wrong value is not desirable. Could you keep it as an error? Or make the field an Option?

kornelski · 2025-09-30T21:51:44Z

unencrypted H/2 theoretically exists, so this downgrade isn't obvious. Maybe the no_tls option could have a different name? or tls and h2 have their own options?

I was having trouble with unencrypted http2 working with the cloudflare endopints. I'll try again, but @fisherdarling has agreed that might be the case

I absolutely expect that unencrypted H/2 will be difficult to use.

I'm mainly worried about H/1 vs H/2+TLS changing two variables at once, from UI perspective. In case somebody wanted to compare TLS overhead, it isn't apples to apples comparison.

So the toggle could be just clearer about what changes, maybe --plain-h1.

Or if the two could be toggled separately, H/1+TLS would be a possibile option.

kornelski · 2025-09-30T21:52:44Z

plain_http_mode and no_tls mix terminology

-Original file line number
+Diff line change
@@ Expand Up / @@ -27,4 +27,5 @@ serde_json = { workspace = true } @@
     tokio = { workspace = true, features = ["rt-multi-thread", "time", "net", "macros",] }
     tokio-util = { workspace = true }
     tracing = { workspace = true }
-    tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] }
+    tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] }
+    url = { workspace = true }

-Original file line number
+Diff line change
@@ Expand Up / @@ -16,6 +16,7 @@ use nq_tokio_network::TokioNetwork; @@
     use serde::{Deserialize, Serialize};
     use tokio_util::sync::CancellationToken;
     use tracing::debug;
+    use url::Url;
     use crate::util::{pretty_ms, pretty_secs_to_ms};
@@ Expand Down Expand Up / @@ -84,22 +85,31 @@ impl CloudflareAimResults { @@
         pub async fn upload(&self) -> anyhow::Result<()> {
             let results = self.clone();
-            let origin = self.origin.clone();
+            // Always report over HTTPS regardless of --no-tls test mode. If an http origin was
+            // provided (should not normally happen), rewrite scheme to https for the upload.
+            let mut origin = self.origin.clone();
+        if let Ok(mut parsed) = Url::parse(&origin) {
+                if parsed.scheme() != "https" {
+                    let _ = parsed.set_scheme("https");
+                    origin = parsed.to_string();
+                }
+            }
             let shutdown = CancellationToken::new();
             let time = Arc::new(TokioTime::new());
             let network = Arc::new(TokioNetwork::new(
                 Arc::clone(&time) as Arc<dyn Time>,
                 shutdown,
+                false,
             ));
             let mut headers = HeaderMap::new();
             headers.append("Origin", HeaderValue::from_str(origin.as_str()).unwrap());
             headers.append("Content-Type", HeaderValue::from_static("application/json"));
             let body = serde_json::to_string(&results).unwrap();
+            // Force a fresh H2 (TLS) connection for AIM upload.
             let response = Client::default()
-                .new_connection(nq_core::ConnectionType::H2)
                 .new_connection(nq_core::ConnectionType::H2)
                 .headers(headers)
                 .method("POST")
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -69,6 +69,10 @@ pub struct RpmArgs { @@
         /// https://blog.cloudflare.com/aim-database-for-internet-quality/
         #[clap(long)]
         pub disable_aim_scores: bool,
+        /// Disable TLS for H1 connections (plain TCP). When set, HTTP/1.1 is used
+        /// without a TLS handshake; otherwise HTTP/2 is preferred.
+        #[clap(long = "no-tls")]
+        pub no_tls: bool,
     }
     impl Default for RpmArgs {
@@ Expand All / @@ -86,6 +90,7 @@ impl Default for RpmArgs { @@
                 interval_duration_ms: 1000, // 1s
                 test_duration_ms: 12_000,   // 12s
                 disable_aim_scores: false,
+                no_tls: false,
             }
         }
     }

-Original file line number
+Diff line change
@@ Expand Up @@
         let result = run_test(&LatencyConfig {
             url: url.parse()?,
             runs,
+            no_tls: false,
         })
         .await?;
@@ Expand All @@
         let network = Arc::new(TokioNetwork::new(
             Arc::clone(&time),
             shutdown.clone(),
+            config.no_tls,
         )) as Arc<dyn Network>;
         let rtt = Latency::new(config.clone());
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -49,6 +49,7 @@ async fn fetch_turn_server_creds( @@
         let network = Arc::new(TokioNetwork::new(
             Arc::clone(&time) as Arc<dyn Time>,
             shutdown.clone(),
+            false,
         ));
         let host = config
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow rpm test to run in non-tls mode #31

Diff view

Diff view

There are no files selected for viewing

Uh oh!

kornelski Sep 30, 2025

Uh oh!

kornelski Sep 30, 2025

Uh oh!

kornelski Sep 30, 2025

Uh oh!

kornelski Sep 30, 2025

Uh oh!

BenMatase Oct 1, 2025

Uh oh!

kornelski Oct 2, 2025

Uh oh!

kornelski Sep 30, 2025

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -55,13 +55,18 @@ struct RpmReport { @@
     impl RpmReport {
         pub fn from_rpm_result(result: &ResponsivenessResult) -> anyhow::Result<RpmReport> {
+            let throughput = result.throughput().context("no throughputs available")?;
+            let loaded_latency_ms = match result.self_probe_latencies.quantile(0.5).map(pretty_ms) {
+                Some(v) => v,
+                None => {
+                    tracing::warn!("no loaded latency measurements; defaulting to 0ms");
+.0
+                }
+            };
             Ok(RpmReport {
-                throughput: result.throughput().context("no throughputs available")?,
-                loaded_latency_ms: result
-                    .self_probe_latencies
-                    .quantile(0.5)
-                    .map(pretty_ms)
-                    .context("no loaded latency measurements")?,
+                throughput,
+                loaded_latency_ms,
                 rpm: result.rpm as usize,
             })
         }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -25,10 +25,11 @@ use crate::util::pretty_secs_to_ms; @@
     pub async fn run(cli_config: RpmArgs) -> anyhow::Result<()> {
         info!("running responsiveness test");
-        let rpm_urls = match cli_config.config.clone() {
+        let mut rpm_urls = match cli_config.config.clone() {
             Some(endpoint) => {
                 info!("fetching configuration from {endpoint}");
-                let urls = get_rpm_config(endpoint).await?.urls;
+                let urls = get_rpm_config(endpoint, cli_config.no_tls).await?.urls;
                 info!("retrieved configuration urls: {urls:?}");
                 urls
@@ Expand All @@
             }
         };
+        // If --no-tls is specified, automatically convert provided HTTPS test URLs to HTTP.
+        // We only rewrite schemes; host, path, query, fragment remain unchanged.
+        if cli_config.no_tls {
+            let downgrade = |orig: &str| -> String {
+                if let Ok(mut url) = url::Url::parse(orig) {
+                    if url.scheme() == "https" {
+                        // Ignore result of set_scheme (fails only if new scheme invalid length per spec).
+                        let _ = url.set_scheme("http");
+                        return url.to_string();
+                    }
+                }
+                orig.to_string()
+            };
+            let original = rpm_urls.clone();
+            rpm_urls.small_https_download_url = downgrade(&rpm_urls.small_https_download_url);
+            rpm_urls.large_https_download_url = downgrade(&rpm_urls.large_https_download_url);
+            rpm_urls.https_upload_url = downgrade(&rpm_urls.https_upload_url);
+            info!(
+                "converted urls for --no-tls: small: {} -> {}, large: {} -> {}, upload: {} -> {}",
+                original.small_https_download_url,
+                rpm_urls.small_https_download_url,
+                original.large_https_download_url,
+                rpm_urls.large_https_download_url,
+                original.https_upload_url,
+                rpm_urls.https_upload_url
+            );
+        }
         // first get unloaded RTT measurements
         info!("determining unloaded latency");
         let rtt_result = crate::latency::run_test(&LatencyConfig {
             url: rpm_urls.small_https_download_url.parse()?,
             runs: 20,
+            no_tls: cli_config.no_tls,
         })
         .await?;
         info!(
@@ Expand All @@
             trimmed_mean_percent: cli_config.trimmed_mean_percent,
             std_tolerance: cli_config.std_tolerance,
             max_loaded_connections: cli_config.max_loaded_connections,
+            no_tls: cli_config.no_tls,
         };
         info!("running download test");
@@ Expand Down Expand Up / @@ -120,6 +152,7 @@ async fn run_test( @@
         let network = Arc::new(TokioNetwork::new(
             Arc::clone(&time),
             shutdown.clone().into(),
+            config.no_tls,
         )) as Arc<dyn Network>;
         let rpm = Responsiveness::new(config.clone(), download)?;
@@ Expand All / @@ -139,7 +172,7 @@ pub struct RpmServerConfig { @@
         urls: RpmUrls,
     }
-    #[derive(Debug, Serialize, Deserialize)]
+    #[derive(Debug, Serialize, Deserialize, Clone)]
     pub struct RpmUrls {
         #[serde(alias = "small_download_url")]
         small_https_download_url: String,
@@ Expand All / @@ -149,16 +182,19 @@ pub struct RpmUrls { @@
         https_upload_url: String,
     }
-    pub async fn get_rpm_config(config_url: String) -> anyhow::Result<RpmServerConfig> {
+    pub async fn get_rpm_config(config_url: String, no_tls: bool) -> anyhow::Result<RpmServerConfig> {
         let shutdown = CancellationToken::new();
         let time = Arc::new(TokioTime::new());
         let network = Arc::new(TokioNetwork::new(
             Arc::clone(&time) as Arc<dyn Time>,
             shutdown.clone(),
+            no_tls,
         ));
+        let conn_type = if no_tls { ConnectionType::H1 } else { ConnectionType::H2 };
         let response = Client::default()
-            .new_connection(ConnectionType::H2)
+            .plain_http_mode(no_tls)
+            .new_connection(conn_type)
             .method("GET")
             .send(
                 config_url.parse().context("parsing rpm config url")?,
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -20,8 +20,7 @@ use serde_json::json; @@
     pub async fn download(args: DownloadArgs) -> anyhow::Result<()> {
         let shutdown = CancellationToken::new();
         let time = Arc::new(TokioTime::new()) as Arc<dyn Time>;
-        let network =
-            Arc::new(TokioNetwork::new(Arc::clone(&time), shutdown.clone())) as Arc<dyn Network>;
+        let network = Arc::new(TokioNetwork::new(Arc::clone(&time), shutdown.clone(), false)) as Arc<dyn Network>;
         let conn_type = match args.conn_type {
             ConnType::H1 => ConnectionType::H1,
@@ Expand Down Expand Up @@
     pub async fn upload(args: UploadArgs) -> anyhow::Result<()> {
         let shutdown = CancellationToken::new();
         let time = Arc::new(TokioTime::new()) as Arc<dyn Time>;
-        let network =
-            Arc::new(TokioNetwork::new(Arc::clone(&time), shutdown.clone())) as Arc<dyn Network>;
+        let network = Arc::new(TokioNetwork::new(Arc::clone(&time), shutdown.clone(), false)) as Arc<dyn Network>;
         let conn_type = match args.conn_type {
             ConnType::H1 => ConnectionType::H1, // ConnectionType::H1,
@@ Expand Down @@

Allow rpm test to run in non-tls mode #31

Are you sure you want to change the base?

Allow rpm test to run in non-tls mode #31

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

kornelski Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

kornelski Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

kornelski Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

kornelski Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

BenMatase Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

kornelski Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

kornelski Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!