tonic-prost-build: URLs in service method comments are not escaped for proper Rust doc formats

[andrew-signal]

Bug Report

Version

tonic v0.14.6
tonic-build v0.14.6
tonic-prost v0.14.6
tonic-prost-build v0.14.6
prost-build v0.14.4

This also happens on the current grpc-rust master branch.

Platform

Linux x86_64 on GitHub Actions. Also reproduced on macOS arm64.

Crates

tonic-prost-build and tonic-build

Description

tonic-prost-build copies bare URLs from RPC method comments into Rust doc comments. This makes cargo doc fail when rustdoc::bare_urls is denied. The default cleanup-markdown feature does not fix it.

Here is a rough sketch for a minimal reproducer:

   syntax = "proto3";

   package repro;

   service Reproducer {
     // https://example.com/
     rpc Call(Request) returns (Response) {}
   }

   message Request {}
   message Response {}

The build script is:

  fn main() {
      tonic_prost_build::configure()
          .build_server(false)
          .build_transport(false)
          .compile_protos(&["proto/repro.proto"], &["proto"])
          .unwrap();
  }

The library is:

  #![deny(rustdoc::bare_urls)]

  tonic::include_proto!("repro");

Running cargo doc fails because the generated file contains a bare link in the doc-comment on that method on the generated service, like so:

  /// https://example.com/
  pub async fn call(/* ... */) {
      // ...
  }

I expected the generated comment to contain an automatic Markdown escaped URL instead, so that it passes Rust doc style checks.

/// <https://example.com/>
pub async fn call(/* ... */) {
  // ...
}

Diagnostics

From what I can work out, prost-build already does this cleanup for message comments via prost's built in sanitization and formatting. Where this goes wrong is that tonic registers/implements its own the Generator for generating its own services, and then that handler feeds that doc-comment without formatting/sanitization straight into the Rust docs.

I can imagine and did implement a very rough test fix, right in the generator:

diff --git a/tonic-prost-build/Cargo.toml b/tonic-prost-build/Cargo.toml
index fd1efc2..9ced3e8 100644
--- a/tonic-prost-build/Cargo.toml
+++ b/tonic-prost-build/Cargo.toml
@@ -24,6 +24,7 @@ prost-types = { version = "0.14" }
 prettyplease = { version = "0.2" }
 proc-macro2 = "1.0"
 quote = "1.0"
+regex = "1"
 syn = "2.0"
 tempfile = "3.0"
 
diff --git a/tonic-prost-build/src/lib.rs b/tonic-prost-build/src/lib.rs
index 055b511..da25762 100644
--- a/tonic-prost-build/src/lib.rs
+++ b/tonic-prost-build/src/lib.rs
@@ -363,8 +363,38 @@ impl ServiceGenerator {
     }
 }
 
+/// Sanitizes comment lines for rustdoc the same way prost-build does for
+/// message and field comments (`Comments::sanitize_line`, which is private):
+/// - escape bare URLs as `<http://foo.com>`
+/// - escape `[` & `]` if not already escaped and not part of a markdown link
+fn sanitize_comments(comments: &mut prost_build::Comments) {
+    use regex::Regex;
+    use std::sync::LazyLock;
+
+    static RULE_URL: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"https?://[^\s)]+").unwrap());
+    static RULE_BRACKETS: LazyLock<Regex> =
+        LazyLock::new(|| Regex::new(r"(^|[^\]\\])\[(([^\]]*[^\\])?)\]([^(\[]|$)").unwrap());
+
+    let sanitize_line = |line: &mut String| {
+        let mut s = RULE_URL.replace_all(line, r"<$0>").to_string();
+        s = RULE_BRACKETS.replace_all(&s, r"$1\[$2\]$4").to_string();
+        *line = s;
+    };
+
+    for block in &mut comments.leading_detached {
+        block.iter_mut().for_each(sanitize_line);
+    }
+    comments.leading.iter_mut().for_each(sanitize_line);
+    comments.trailing.iter_mut().for_each(sanitize_line);
+}
+
 impl prost_build::ServiceGenerator for ServiceGenerator {
-    fn generate(&mut self, service: Service, buf: &mut String) {
+    fn generate(&mut self, mut service: Service, buf: &mut String) {
+        sanitize_comments(&mut service.comments);
+        for method in &mut service.methods {
+            sanitize_comments(&mut method.comments);
+        }
+
         let tonic_service = TonicBuildService::new(service, self.codec_path.clone());
 
         let mut builder = CodeGenBuilder::new();

This resolves the problem, but I'm not sure how naturally it lends itself to a production fix. The real fix is probably to adjust the visibility of sanitize_line in prost_build, maybe with some ownership model tweaks, and then adjust how we actually return the comments to be sanitized?