Merge pull request #128 from tommilligan/bug-127

markdown: fix panic when searching for indent

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+## 1.12.1
+
+### Fixed
+
+- Panic when searching for an indent in non-ASCII content. Thanks to [@CoralPink](https://github.com/CoralPink) for the report! ([#128](https://github.com/tommilligan/mdbook-admonish/pull/128)
+
 ## 1.12.0
 
 ### Added

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mdbook-admonish"
-version = "1.12.0"
+version = "1.12.1"
 edition = "2021"
 
 authors = ["Tom Milligan <[email protected]>"]

src/markdown.rs

@@ -28,18 +28,8 @@ pub(crate) fn preprocess(
     for (event, span) in events.into_offset_iter() {
         if let Event::Start(Tag::CodeBlock(Fenced(info_string))) = event.clone() {
             let span_content = &content[span.start..span.end];
-
-            // Scan for a line start before this span.
-            // For safety, only scan up to a fixed limit of the text
             const INDENT_SCAN_MAX: usize = 1024;
-            // If there's less text than that, just scan from the start
-            let line_scan_start = span.start.checked_sub(INDENT_SCAN_MAX).unwrap_or_default();
-            // If we can't find a newline, assume no indent
-            let indent = content[line_scan_start..span.start]
-                .chars()
-                .rev()
-                .position(|c| c == '\n')
-                .unwrap_or_default();
+            let indent = indent_of(content, span.start, INDENT_SCAN_MAX);
 
             let admonition = match parse_admonition(
                 info_string.as_ref(),
@@ -75,11 +65,73 @@ pub(crate) fn preprocess(
     Ok(content)
 }
 
+/// Returns the indent of the given position.
+///
+/// Defined as the number of characters between the given `position` (where
+/// position is a valid char boundary byte-index in `content`),
+/// and the previous newline character `\n`.
+///
+/// `max` is the maximum number of characters to scan before assuming there is
+/// no indent (will return zero if exceeded).
+///
+/// ## Panics
+///
+/// Will panic if `position` is not a valid utf-8 char boundary index of `content`.
+fn indent_of(content: &str, position: usize, max: usize) -> usize {
+    // Scan for a line start before this span.
+    content[..position]
+        .chars()
+        .rev()
+        // For safety, only scan up to a fixed limit of the text
+        .take(max)
+        .position(|c| c == '\n')
+        // If we can't find a newline, assume no indent
+        .unwrap_or_default()
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
     use pretty_assertions::assert_eq;
 
+    #[test]
+    fn indent_of_samples() {
+        for (content, position, max, expected) in [
+            // Empty case
+            ("", 0, 10, 0),
+            ("no newline", 4, 10, 0),
+            // Newline at position 5, difference from 8 is 3
+            ("with\nnewline", 8, 10, 3),
+            // If no newline in safety limit, return 0
+            ("with\nnewline", 8, 2, 0),
+            // Safety limit is characters, not bytes
+            // Regression test for FIXME LINK
+            (
+                "例えばこれは",
+                // Position is generated by mdbook internals, should be valid char limit
+                // This mimics the second character starting the span
+                "例".len(),
+                // Any arbitrary safetly limit should be valid
+                1,
+                // Should not panic
+                0,
+            ),
+            (
+                "例え\n  れは",
+                // Position is generated by mdbook internals, should be valid char limit
+                // This mimics the second character starting the span
+                "例え\n  ".len(),
+                // Any arbitrary safetly limit should be valid
+                4,
+                // Should not panic
+                2,
+            ),
+        ] {
+            let actual = indent_of(content, position, max);
+            assert_eq!(actual, expected);
+        }
+    }
+
     fn prep(content: &str) -> String {
         preprocess(
             content,