Repository: pulldown-cmark/pulldown-cmark (rust)

Issue:
[fuzz result] false positive inline HTML within blockquote
In this sample doc, there definitely shouldn't be any inline HTML, because inline HTML tags are required to have an `>` at the end. The `>` in the document is part of the block quote, not the HTML:

```markdown
><span
title
>junk
```

Events from pulldown-cmark:

```rust
"><V\nT\n>T" -> [
  Start(BlockQuote)
    Start(Paragraph)
      Html(Boxed("<V\nT"))
      Text(Borrowed("T"))
    End(Paragraph)
  End(BlockQuote)
]
```

Events from commonmark.js:

```rust
"><V\nT\n>T" -> [
  Start(BlockQuote)
    Start(Paragraph)
      Text(Boxed("<V"))
      SoftBreak
      Text(Boxed("T"))
      SoftBreak
      Text(Boxed("T"))
    End(Paragraph)
  End(BlockQuote)
]
```

XML from [commonmark.js](https://spec.commonmark.org/dingus/?text=%3E%3CV%0AT%0A%3ET):

```xml
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE document SYSTEM "CommonMark.dtd">
<document xmlns="http://commonmark.org/xml/1.0">
  <block_quote>
    <paragraph>
      <text>&lt;</text>
      <text>V</text>
      <softbreak />
      <text>T</text>
      <softbreak />
      <text>T</text>
    </paragraph>
  </block_quote>
</document>
```


Pull request:
Don't exit `scan_attribute` with the ix pointing at block quote
Fixes #857

That would be ambiguous, because it can't easily be distinguished between the block quote `>` and the end of an HTML tag `>`. If this function bails out because it can't find an `=`, it should point at the spot immedatiely after the attribute name instead.

Patch:
diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs
index f453dc5..2a4b675 100644
--- a/fuzz/src/lib.rs
+++ b/fuzz/src/lib.rs
@@ -15,11 +15,7 @@ use pulldown_cmark::{CodeBlockKind, Event, LinkType, Parser, Tag, TagEnd};
 use quick_xml::escape::unescape;
 use quick_xml::events::Event as XmlEvent;
 use quick_xml::reader::Reader;
-
-fn urldecode(data: &str) -> String {
-    let decoded = urlencoding::decode_binary(data.as_bytes());
-    urlencoding::encode_binary(&decoded[..]).to_string()
-}
+use urlencoding::decode as urldecode;
 
 /// Send Markdown `text` to `pulldown-cmark` and return Markdown
 /// events.
@@ -175,12 +171,13 @@ pub fn xml_to_events(xml: &str) -> anyhow::Result<Vec<Event>> {
                         .into(),
                 )),
                 name @ (b"link" | b"image") => {
-                    let dest_url = tag
+                    let url_encoded_dest_url = tag
                         .try_get_attribute("destination")?
                         .ok_or(anyhow!("Missing destination"))?
-                        .unescape_value()?
-                        .into_owned()
-                        .into();
+                        .unescape_value()?;
+                    let dest_url = urldecode(&url_encoded_dest_url)
+                        .unwrap_or_else(|_| url_encoded_dest_url.clone().into())
+                        .into_owned().into();
                     let title = match tag.try_get_attribute("title")? {
                         Some(title) => title.unescape_value()?.into_owned().into(),
                         None => "".into(),
@@ -318,7 +315,7 @@ pub fn normalize(events: Vec<Event<'_>>) -> Vec<Event<'_>> {
                 ..
             }) => Some(Event::Start(Tag::Link {
                 link_type: LinkType::Inline,
-                dest_url: urldecode(&format!("mailto:{dest_url}")).into(),
+                dest_url: format!("mailto:{dest_url}").into(),
                 title: title.clone(),
                 id: "".into(), // commonmark.js does not record this
             })),
@@ -328,7 +325,7 @@ pub fn normalize(events: Vec<Event<'_>>) -> Vec<Event<'_>> {
                 ..
             }) => Some(Event::Start(Tag::Link {
                 link_type: LinkType::Inline,
-                dest_url: urldecode(&dest_url).into(),
+                dest_url: dest_url.clone(),
                 title: title.clone(),
                 id: "".into(), // commonmark.js does not record this
             })),
@@ -340,7 +337,7 @@ pub fn normalize(events: Vec<Event<'_>>) -> Vec<Event<'_>> {
                 ..
             }) => Some(Event::Start(Tag::Image {
                 link_type: LinkType::Inline,
-                dest_url: urldecode(&dest_url).into(),
+                dest_url: dest_url.clone(),
                 title: title.clone(),
                 id: id.clone(),
             })),
diff --git a/pulldown-cmark/specs/regression.txt b/pulldown-cmark/specs/regression.txt
index 5aca680..92547b0 100644
--- a/pulldown-cmark/specs/regression.txt
+++ b/pulldown-cmark/specs/regression.txt
@@ -2511,3 +2511,17 @@ ISSUE 853
 .
 <p><a href="((()))">linky</a></p>
 ````````````````````````````````
+
+ISSUE 857
+
+```````````````````````````````` example
+><span
+title
+>junk
+.
+<blockquote>
+<p>&lt;span
+title
+junk</p>
+</blockquote>
+````````````````````````````````
diff --git a/pulldown-cmark/src/scanners.rs b/pulldown-cmark/src/scanners.rs
index 728d233..73bf33a 100644
--- a/pulldown-cmark/src/scanners.rs
+++ b/pulldown-cmark/src/scanners.rs
@@ -915,18 +915,18 @@ fn scan_attribute(
     buffer_ix: &mut usize,
 ) -> Option<usize> {
     ix += scan_attribute_name(&data[ix..])?;
-    let n_whitespace =
-        scan_whitespace_with_newline_handler(data, ix, newline_handler, buffer, buffer_ix)? - ix;
-    ix += n_whitespace;
+    let ix_after_attribute = ix;
+    ix = scan_whitespace_with_newline_handler_without_buffer(data, ix, newline_handler)?;
     if scan_ch(&data[ix..], b'=') == 1 {
+        ix = scan_whitespace_with_newline_handler(data, ix_after_attribute, newline_handler, buffer, buffer_ix)?;
         ix += 1;
         ix = scan_whitespace_with_newline_handler(data, ix, newline_handler, buffer, buffer_ix)?;
         ix = scan_attribute_value(data, ix, newline_handler, buffer, buffer_ix)?;
-    } else if n_whitespace > 0 {
+        Some(ix)
+    } else {
         // Leave whitespace for next attribute.
-        ix -= 1;
+        Some(ix_after_attribute)
     }
-    Some(ix)
 }
 
 /// Scans whitespace and possibly newlines according to the
@@ -962,6 +962,35 @@ fn scan_whitespace_with_newline_handler(
     Some(i)
 }
 
+/// Scans whitespace and possible newlines according to the behavior defined
+/// by the newline handler.
+///
+/// Unlike [`scan_whitespace_with_newline_handler`], this function doesn't
+/// copy skipped data into a buffer. Typically, if this function
+/// returns `Some`, a call to `scan_whitespace_with_newline_handler` will
+/// soon follow.
+fn scan_whitespace_with_newline_handler_without_buffer(
+    data: &[u8],
+    mut i: usize,
+    newline_handler: Option<&dyn Fn(&[u8]) -> usize>,
+) -> Option<usize> {
+    while i < data.len() {
+        if !is_ascii_whitespace(data[i]) {
+            return Some(i);
+        }
+        if let Some(eol_bytes) = scan_eol(&data[i..]) {
+            let handler = newline_handler?;
+            i += eol_bytes;
+            let skipped_bytes = handler(&data[i..]);
+            i += skipped_bytes;
+        } else {
+            i += 1;
+        }
+    }
+
+    Some(i)
+}
+
 /// Returns the index immediately following the attribute value on success.
 fn scan_attribute_value(
     data: &[u8],


Test patch:
diff --git a/pulldown-cmark/tests/suite/regression.rs b/pulldown-cmark/tests/suite/regression.rs
index d5e1fad..75660e8 100644
--- a/pulldown-cmark/tests/suite/regression.rs
+++ b/pulldown-cmark/tests/suite/regression.rs
@@ -3000,3 +3000,19 @@ fn regression_test_189() {
 
     test_markdown_html(original, expected, false, false, false);
 }
+
+#[test]
+fn regression_test_190() {
+    let original = r##"><span
+title
+>junk
+"##;
+    let expected = r##"<blockquote>
+<p>&lt;span
+title
+junk</p>
+</blockquote>
+"##;
+
+    test_markdown_html(original, expected, false, false, false);
+}