Fix not correctly encoding web viewer URLs (#11128)

`ViewerOpenURL::sharable_url` didn't correctly encode the parameters for
web urls. That works for surprisingly many urls just fine, but once you
start adding fragments & parameters within those URLs it naturally
breaks down!

Fixes RR-2225

Author: Wumpf

crates/viewer/re_viewer/src/open_url.rs

@@ -370,13 +370,14 @@ impl ViewerOpenUrl {
         // Combine the URL(s) with the web viewer base URL if provided.
         if let Some(web_viewer_base_url) = web_viewer_base_url {
             let mut share_url = web_viewer_base_url.clone();
-            share_url.set_query(Some(
-                &urls
-                    .into_iter()
-                    .map(|url| format!("url={url}"))
-                    .collect::<Vec<_>>()
-                    .join("&"),
-            ));
+
+            // Use the form_urlencoded::Serializer to build the query string with multiple "url" parameters.
+            // It's important to not just append the strings, since we have to take care of correctly escaping.
+            let mut serializer = url::form_urlencoded::Serializer::new(String::new());
+            for url in &urls {
+                serializer.append_pair("url", url);
+            }
+            share_url.set_query(Some(&serializer.finish()));
 
             Ok(share_url.to_string())
         } else if urls.len() == 1 {
@@ -978,60 +979,58 @@ mod tests {
             ViewerOpenUrl::IntraRecordingSelection("my/path".parse().unwrap())
                 .sharable_url(base_url_param)
                 .unwrap(),
-            "https://foo.com/test?url=recording://my/path"
-        );
-
-        assert_eq!(
-            ViewerOpenUrl::RrdHttpUrl(Url::parse("https://example.com/data.rrd").unwrap())
-                .sharable_url(base_url_param)
-                .unwrap(),
-            "https://foo.com/test?url=https://example.com/data.rrd"
+            "https://foo.com/test?url=recording%3A%2F%2Fmy%2Fpath"
         );
 
         assert_eq!(
-            ViewerOpenUrl::FilePath("/path/to/file.rrd".into())
+            ViewerOpenUrl::RrdHttpUrl("https://example.com/data.rrd".parse().unwrap())
                 .sharable_url(base_url_param)
                 .unwrap(),
-            "https://foo.com/test?url=/path/to/file.rrd"
+            "https://foo.com/test?url=https%3A%2F%2Fexample.com%2Fdata.rrd"
         );
 
-        let uri =
-            "rerun://127.0.0.1:1234/dataset/1830B33B45B963E7774455beb91701ae?partition_id=pid";
         assert_eq!(
-            ViewerOpenUrl::RedapDatasetPartition(uri.parse().unwrap())
-                .sharable_url(base_url_param)
-                .unwrap(),
-            format!("https://foo.com/test?url={uri}")
+            ViewerOpenUrl::RedapDatasetPartition(
+                "rerun://127.0.0.1:1234/dataset/1830B33B45B963E7774455beb91701ae?partition_id=pid"
+                    .parse()
+                    .unwrap()
+            )
+            .sharable_url(base_url_param)
+            .unwrap(),
+            format!(
+                "https://foo.com/test?url=rerun%3A%2F%2F127.0.0.1%3A1234%2Fdataset%2F1830B33B45B963E7774455beb91701ae%3Fpartition_id%3Dpid"
+            )
         );
 
         assert_eq!(
             ViewerOpenUrl::RedapProxy("rerun://localhost:51234/proxy".parse().unwrap())
                 .sharable_url(base_url_param)
                 .unwrap(),
-            "https://foo.com/test?url=rerun://localhost:51234/proxy"
+            "https://foo.com/test?url=rerun%3A%2F%2Flocalhost%3A51234%2Fproxy"
         );
 
         assert_eq!(
             ViewerOpenUrl::RedapCatalog("rerun://localhost:51234/catalog".parse().unwrap())
                 .sharable_url(base_url_param)
                 .unwrap(),
-            "https://foo.com/test?url=rerun://localhost:51234/catalog"
+            "https://foo.com/test?url=rerun%3A%2F%2Flocalhost%3A51234%2Fcatalog"
         );
 
         let entry_id = EntryId::new();
         let url = format!("rerun://localhost:51234/entry/{entry_id}");
+        let encoded_url = url::form_urlencoded::byte_serialize(url.as_bytes()).collect::<String>();
         assert_eq!(
             ViewerOpenUrl::RedapEntry(url.parse().unwrap())
                 .sharable_url(base_url_param)
                 .unwrap(),
-            format!("https://foo.com/test?url={url}")
+            format!("https://foo.com/test?url={encoded_url}")
         );
 
         assert_eq!(
             ViewerOpenUrl::WebEventListener
                 .sharable_url(base_url_param)
                 .unwrap(),
-            "https://foo.com/test?url=web_event:"
+            "https://foo.com/test?url=web_event%3A"
         );
 
         assert_eq!(
@@ -1043,7 +1042,7 @@ mod tests {
             }
             .sharable_url(base_url_param)
             .unwrap(),
-            "https://foo.com/test?url=https://example.com/data.rrd",
+            "https://foo.com/test?url=https%3A%2F%2Fexample.com%2Fdata.rrd",
         );
         assert_eq!(
             ViewerOpenUrl::WebViewerUrl {
@@ -1055,7 +1054,7 @@ mod tests {
             }
             .sharable_url(base_url_param)
             .unwrap(),
-            "https://foo.com/test?url=https://example.com/bar.rrd&url=rerun://localhost:51234/proxy",
+            "https://foo.com/test?url=https%3A%2F%2Fexample.com%2Fbar.rrd&url=rerun%3A%2F%2Flocalhost%3A51234%2Fproxy",
         );
     }
 }