Repository: zkat/miette
Pull request #393 (2024-08-05 01:26:43)

Handle invalid UTF-8 in source code
At the moment if invalid UTF-8 is passed to the source code, then the `Report` will panic when rendered. This PR changes it to properly decode UTF-8 and handle the invalid data in the usual way (i.e. they will be replaced with �).

This can require an extra allocation when rendering `Report`, but only for invalid UTF-8, and only for the "context" area, which is usually small (several lines).

This is simply a one-line change, but there are tests to double-check the behaviour of offsets into invalid source. I added some tests for labels into "good" Unicode source and also into "bad" Unicode source. Both seem to behave as expected.

zkat: what's the deal with clippy. sigh.
waywardmonkeys: @zkat I've got clippy fixed in PR #395.
zkat: awesome thanks. You can rebase now.
Porges: > awesome thanks. You can rebase now.

Fixed the 1.70 build as well. Should be good to go.

Patch:
diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs
index 87fb354..5991890 100644
--- a/src/handlers/graphical.rs
+++ b/src/handlers/graphical.rs
@@ -1186,7 +1186,7 @@ impl GraphicalReportHandler {
         let context_data = source
             .read_span(context_span, self.context_lines, self.context_lines)
             .map_err(|_| fmt::Error)?;
-        let context = std::str::from_utf8(context_data.data()).expect("Bad utf8 detected");
+        let context = String::from_utf8_lossy(context_data.data());
         let mut line = context_data.line();
         let mut column = context_data.column();
         let mut offset = context_data.span().offset();


Test patch:
diff --git a/tests/graphical.rs b/tests/graphical.rs
index 7221a70..3176812 100644
--- a/tests/graphical.rs
+++ b/tests/graphical.rs
@@ -2349,3 +2349,164 @@ Error: oops::my::inner
     assert_eq!(expected, &out);
     Ok(())
 }
+
+#[test]
+fn after_unicode_width() -> Result<(), MietteError> {
+    #[derive(Debug, Diagnostic, Error)]
+    #[error("pointing")]
+    #[diagnostic(code(pointing_at))]
+    struct E {
+        #[label("something of interest")]
+        src: SourceSpan,
+    }
+
+    let unicode_source = "höööt!";
+
+    // make err pointing at the t
+    let (t_index, _) = unicode_source
+        .bytes()
+        .enumerate()
+        .find(|&(_, x)| x == b't')
+        .unwrap();
+
+    let err = E {
+        src: SourceSpan::from((t_index, 1)),
+    };
+
+    let result = fmt_report(Report::new(err).with_source_code(String::from(unicode_source)));
+
+    let expected = "pointing_at
+
+  × pointing
+   ╭────
+ 1 │ höööt!
+   ·     ┬
+   ·     ╰── something of interest
+   ╰────
+";
+
+    assert_eq!(expected, result);
+
+    Ok(())
+}
+
+#[test]
+fn at_unicode_width() -> Result<(), MietteError> {
+    #[derive(Debug, Diagnostic, Error)]
+    #[error("pointing")]
+    #[diagnostic(code(pointing_at))]
+    struct E {
+        #[label("something of interest")]
+        src: SourceSpan,
+    }
+
+    let unicode_source = "höööt!";
+
+    // make err pointing at an ö
+    let err = E {
+        src: SourceSpan::from((1, "ö".len())),
+    };
+
+    // we want to make sure the pointer is one char wide, not 2
+    assert!(err.src.len() == 2);
+
+    let result = fmt_report(Report::new(err).with_source_code(String::from(unicode_source)));
+
+    let expected = "pointing_at
+
+  × pointing
+   ╭────
+ 1 │ höööt!
+   ·  ┬
+   ·  ╰── something of interest
+   ╰────
+";
+
+    assert_eq!(expected, result);
+
+    Ok(())
+}
+
+#[test]
+fn at_invalid_unicode() -> Result<(), MietteError> {
+    #[derive(Debug, Diagnostic, Error)]
+    #[error("decoding error")]
+    #[diagnostic(code(decode_err))]
+    struct E {
+        #[label("invalid data here")]
+        src: SourceSpan,
+    }
+
+    // 3 bytes here are mapped to 1 char when replaced by replacement character
+    // this tests that the line pointing from the label is the correct length when rendered
+    // - it should be 1 char wide not 3 chars
+    let invalid_source: &[u8] = b"malformed h\xf0\x93\x8aXYZ";
+
+    // invalid utf8 lint only available from 1.72
+    #[allow(unknown_lints, invalid_from_utf8)]
+    let utf8_err = std::str::from_utf8(invalid_source).unwrap_err();
+
+    // make err pointing at the invalid part
+    let err = E {
+        src: SourceSpan::from((utf8_err.valid_up_to(), utf8_err.error_len().unwrap_or(1))),
+    };
+
+    // check that we're testing the thing we think we're testing
+    assert_eq!(err.src.len(), 3);
+
+    let result = fmt_report(Report::new(err).with_source_code(Vec::from(invalid_source)));
+
+    let expected = "decode_err
+
+  × decoding error
+   ╭────
+ 1 │ malformed h�XYZ
+   ·            ┬
+   ·            ╰── invalid data here
+   ╰────
+";
+
+    assert_eq!(expected, result);
+
+    Ok(())
+}
+
+#[test]
+fn after_invalid_unicode() -> Result<(), MietteError> {
+    #[derive(Debug, Diagnostic, Error)]
+    #[error("decoding error")]
+    #[diagnostic(code(decode_err))]
+    struct E {
+        #[label("valid data here")]
+        src: SourceSpan,
+    }
+
+    let invalid_source: &[u8] = b"malformed h\xf0\x93\x8aXYZ";
+
+    let (x_index, _) = invalid_source
+        .iter()
+        .enumerate()
+        .find(|&(_, &x)| x == b'X')
+        .unwrap();
+
+    // make err pointing at the X
+    let err = E {
+        src: SourceSpan::from((x_index, 1)),
+    };
+
+    let result = fmt_report(Report::new(err).with_source_code(invalid_source));
+
+    let expected = "decode_err
+
+  × decoding error
+   ╭────
+ 1 │ malformed h�XYZ
+   ·             ┬
+   ·             ╰── valid data here
+   ╰────
+";
+
+    assert_eq!(expected, result);
+
+    Ok(())
+}