WIP: failsafe to prevent infinite loop, more tests

Author: lolbinarycat

src/librustdoc/lib.rs

@@ -11,6 +11,7 @@
 #![feature(file_buffered)]
 #![feature(format_args_nl)]
 #![feature(if_let_guard)]
+#![feature(iter_advance_by)]
 #![feature(iter_intersperse)]
 #![feature(round_char_boundary)]
 #![feature(rustc_private)]

src/librustdoc/passes/lint/html_tags.rs

@@ -350,8 +350,8 @@ impl TagParser {
                 None => break,
             };
             prev_pos = pos;
-            // Checking if this is a closing tag (like `</a>` for `<a>`).
-            if c == '/' && self.tag_name.is_empty() {
+            if !self.in_attrs && c == '/' && self.tag_name.is_empty() {
+                // Checking if this is a closing tag (like `</a>` for `<a>`).
                 self.is_closing = true;
             } else if !self.in_attrs && is_valid_for_html_tag_name(c, self.tag_name.is_empty()) {
                 self.tag_name.push(c);
@@ -389,69 +389,83 @@ impl TagParser {
                             }
                         }
                         self.drop_tag(r, dox, f);
+                        self.tag_parsed();
                     } else {
-                        let mut is_self_closing = false;
-                        if c != '>' {
-                            'parse_til_gt: {
-                                for (i, c) in text[pos..].char_indices() {
-                                    if !c.is_whitespace() {
-                                        if let Some(q) = self.quote {
-                                            if c == q {
-                                                self.quote = None;
-                                                self.quote_pos = None;
-                                                self.after_eq = false;
-                                            }
-                                        } else if c == '>' {
-                                            // fall through and call `tag_parsed`.
-                                            break 'parse_til_gt;
-                                        } else if c == '<' {
-                                            self.handle_lt_in_tag(range.clone(), pos + i, f);
-                                        } else if c == '/' && !self.after_eq {
-                                            is_self_closing = true;
-                                        } else {
-                                            if is_self_closing {
-                                                is_self_closing = false;
-                                            }
-                                            if (c == '"' || c == '\'') && self.after_eq {
-                                                self.quote = Some(c);
-                                                self.quote_pos = Some(pos + i);
-                                            } else if c == '=' {
-                                                self.after_eq = true;
-                                            }
-                                        }
-                                    } else if self.quote.is_none() {
-                                        self.after_eq = false;
-                                    }
-                                }
-                                // if we've run out of text but still haven't found a `>`,
-                                // break out of the outer loop to skip over `tag_parsed`.
-                                // this allows us to either find the `>` in a later event
-                                // or emit a lint about it being missing.
-                                break 'outer_loop;
-                            }
-                        }
-                        if is_self_closing {
-                            // https://html.spec.whatwg.org/#parse-error-non-void-html-element-start-tag-with-trailing-solidus
-                            let valid = ALLOWED_UNCLOSED.contains(&&self.tag_name[..])
-                                || self.tags.iter().take(pos + 1).any(|(at, _)| {
-                                    let at = at.to_lowercase();
-                                    at == "svg" || at == "math"
-                                });
-                            if !valid {
-                                f(format!("invalid self-closing HTML tag `{}`", self.tag_name), &r, false);
-                            }
-                        } else if !self.tag_name.is_empty() {
-                            self.tags.push((std::mem::take(&mut self.tag_name), r));
-                        }
+                        self.extract_opening_tag(text, range, r, pos, c, iter, f)
                     }
-                    self.tag_parsed();
                 }
                 break;
             }
             iter.next();
         }
     }
 
+    fn extract_opening_tag(
+        &mut self,
+        text: &str,
+        range: &Range<usize>,
+        r: Range<usize>,
+        pos: usize,
+        c: char,
+        iter: &mut Peekable<CharIndices<'_>>,
+        f: &impl Fn(String, &Range<usize>, bool),
+    ) {
+        // we can store this as a local, since html5 does require the `/` and `>`
+        // to not be seperated by whitespace.
+        let mut is_self_closing = false;
+        if c != '>' {
+            'parse_til_gt: {
+                for (i, c) in text[pos..].char_indices() {
+                    if !c.is_whitespace() {
+                        if let Some(q) = self.quote {
+                            if c == q {
+                                self.quote = None;
+                                self.quote_pos = None;
+                                self.after_eq = false;
+                            }
+                        } else if c == '>' {
+                            break 'parse_til_gt;
+                        } else if c == '<' {
+                            self.handle_lt_in_tag(range.clone(), pos + i, f);
+                        } else if c == '/' && !self.after_eq {
+                            is_self_closing = true;
+                        } else {
+                            if is_self_closing {
+                                is_self_closing = false;
+                            }
+                            if (c == '"' || c == '\'') && self.after_eq {
+                                self.quote = Some(c);
+                                self.quote_pos = Some(pos + i);
+                            } else if c == '=' {
+                                self.after_eq = true;
+                            }
+                        }
+                    } else if self.quote.is_none() {
+                        self.after_eq = false;
+                    }
+                }
+                // if we've run out of text but still haven't found a `>`,
+                // return early without calling `tag_parsed` or emitting lints.
+                // this allows us to either find the `>` in a later event
+                // or emit a lint about it being missing.
+                return;
+            }
+        }
+        if is_self_closing {
+            // https://html.spec.whatwg.org/#parse-error-non-void-html-element-start-tag-with-trailing-solidus
+            let valid = ALLOWED_UNCLOSED.contains(&&self.tag_name[..])
+                || self.tags.iter().take(pos + 1).any(|(at, _)| {
+                    let at = at.to_lowercase();
+                    at == "svg" || at == "math"
+                });
+            if !valid {
+                f(format!("invalid self-closing HTML tag `{}`", self.tag_name), &r, false);
+            }
+        } else if !self.tag_name.is_empty() {
+            self.tags.push((std::mem::take(&mut self.tag_name), r));
+        }
+        self.tag_parsed();
+    }
     /// Finished parsing a tag, reset related data.
     fn tag_parsed(&mut self) {
         self.tag_name.clear();
@@ -468,8 +482,23 @@ impl TagParser {
         f: &impl Fn(String, &Range<usize>, bool),
     ) {
         let mut iter = text.char_indices().peekable();
-
-        while let Some((start_pos, c)) = iter.next() {
+        let mut prev_pos = 0;
+        loop {
+            if self.quote.is_some() {
+                assert!(self.in_attrs);
+            }
+            if self.in_attrs &&
+                let Some(&(start_pos, _)) = iter.peek()
+            {
+                self.extract_html_tag(text, &range, dox, start_pos, &mut iter, f);
+                // if no progress is being made, move forward forcefully.
+                if prev_pos == start_pos {
+                    iter.next();
+                }
+                prev_pos = start_pos;
+                continue;
+            }
+            let Some((start_pos, c)) = iter.next() else { break };
             if is_in_comment.is_some() {
                 if text[start_pos..].starts_with("-->") {
                     *is_in_comment = None;
@@ -504,20 +533,46 @@ impl TagParser {
         }
     }
 
+    
+
 }
 
 #[test]
 fn test_extract_tags_nested_unclosed() {
-    use std::rc::Rc;
     use std::cell::RefCell;
-    use std::ops::Deref;
 
     let mut tagp = TagParser::new();
     let mut diags = RefCell::new(Vec::new());
-    let dox = "<div>\n<br<div>";
+    let dox = "<div>\n<br</div>";
     tagp.extract_tags(dox, 0..dox.len(), dox, &mut None, &|s, r, b| {
         diags.borrow_mut().push((s, r.clone(), b));
     });
     assert_eq!(diags.borrow().len(), 1, "did not get expected diagnostics: {diags:?}");
     assert_eq!(diags.borrow()[0].1, 6..9)
 }
+
+#[test]
+fn test_extract_tags_taglike_in_attr() {
+    use std::cell::RefCell;
+
+    let mut tagp = TagParser::new();
+    let mut diags = RefCell::new(Vec::new());
+    let dox = "<img src='<div>'>";
+    tagp.extract_tags(dox, 0..dox.len(), dox, &mut None, &|s, r, b| {
+        diags.borrow_mut().push((s, r.clone(), b));
+    });
+    assert_eq!(diags.borrow().len(), 0, "unexpected diagnostics: {diags:?}");
+}
+
+#[test]
+fn test_extract_tags_taglike_in_multiline_attr() {
+    use std::cell::RefCell;
+
+    let mut tagp = TagParser::new();
+    let mut diags = RefCell::new(Vec::new());
+    let dox = "<img src=\"\nasd\n<div>\n\">";
+    tagp.extract_tags(dox, 0..dox.len(), dox, &mut None, &|s, r, b| {
+        diags.borrow_mut().push((s, r.clone(), b));
+    });
+    assert_eq!(diags.borrow().len(), 0, "unexpected diagnostics: {diags:?}");
+}

tests/rustdoc-ui/lints/invalid-html-tags.rs

@@ -184,16 +184,20 @@ pub fn no_error_5() {}
 /// multiline
 /// attr
 /// values
+/// these are just text, not tags:
+/// </div>
+/// <p/>
+/// <div>
 /// ">
 pub fn no_error_6() {}
 
 /// <a href="data:text/html,<!DOCTYPE>
 /// <html>
 /// <body><b>this is allowed for some reason</b></body>
 /// </html>
-/// ">what</a>'
+/// ">what</a>
 pub fn no_error_7() {}
 
 
 /// <p <!-->all of &gt;, !, and - are valid in attribute names</p>
-pub fn no_error_7() {}
+pub fn no_error_8() {}

tests/rustdoc-ui/lints/invalid-html-tags.stderr

@@ -139,5 +139,41 @@ error: incomplete HTML tag `br`
 LL | /// <br
    |     ^^^
 
-error: aborting due to 22 previous errors
+error: unopened HTML tag `div`
+  --> $DIR/invalid-html-tags.rs:188:5
+   |
+LL | /// </div>
+   |     ^^^^^^
+
+error: invalid self-closing HTML tag `p`
+  --> $DIR/invalid-html-tags.rs:189:5
+   |
+LL | /// <p/>
+   |     ^^
+
+error: unclosed HTML tag `div`
+  --> $DIR/invalid-html-tags.rs:190:5
+   |
+LL | /// <div>
+   |     ^^^^^
+
+error: unopened HTML tag `a`
+  --> $DIR/invalid-html-tags.rs:198:11
+   |
+LL | /// ">what</a>
+   |           ^^^^
+
+error: incomplete HTML tag `p`
+  --> $DIR/invalid-html-tags.rs:202:5
+   |
+LL | /// <p <!-->all of &gt;, !, and - are valid in attribute names</p>
+   |     ^^^
+
+error: Unclosed HTML comment
+  --> $DIR/invalid-html-tags.rs:202:8
+   |
+LL | /// <p <!-->all of &gt;, !, and - are valid in attribute names</p>
+   |        ^^^^
+
+error: aborting due to 28 previous errors