tonic-prost-build: URLs in service method comments are not escaped for proper Rust doc formats
- Dominant language
- Rust
- Stars
- 12.5k
- Forks
- 1.3k
- Avg merge
- 4d 7h
- Merged PRs (30d)
- 24
Description
## 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:
```proto
syntax = "proto3";
package repro;
service Reproducer {
// https://example.com/
rpc Call(Request) returns (Response) {}
}
message Request {}
message Response {}
```
The build script is:
```rust
fn main() {
tonic_prost_build::configure()
.build_server(false)
.build_transport(false)
.compile_protos(&["proto/repro.proto"], &["proto"])
.unwrap();
}
```
The library is:
```rust
#![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:
```rust
/// 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.
```rust
///
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](https://github.com/grpc/grpc-rust/blob/0858621ef6eda0f19928-prost-build/src/lib.rs#L366) for generating its own services, and then that handler [feeds](https://github.com/grpc/grpc-rust/blob/0858621ef6eda0f19928b3972181719f954f5bf2/tonic-build/src/lib.rs#L205) 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
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 ``
+/// - 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 = LazyLock::new(|| Regex::new(r"https?://[^\s)]+").unwrap());
+ static RULE_BRACKETS: LazyLock =
+ 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?
Contributor guide
Research direction
Start in tonic-prost-build/src/lib.rs at ServiceGenerator::generate, then read prost-build's Comments sanitization mentioned in the report. Reproduce the issue with the minimal proto and cargo doc using deny(rustdoc::bare_urls). Done means generated RPC method comments escape bare URLs and cargo doc passes the rustdoc check.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- documentation
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 62/100