Skip to content

Commit

Permalink
fix: fix static str use Box::leak cause memory leak
Browse files Browse the repository at this point in the history
  • Loading branch information
fefit committed Nov 10, 2022
1 parent 1cee95c commit 9a7245e
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 71 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

0.2.1 版本后各个接口方法已经基本趋于稳定,后面将不会做大的调整。

## [0.5.7] - 2022-11-10

### 变更

-`regex``1.4.3` 升级到 `1.7.0` 版本。

- 元素的 `.text()` 及文档 `.source_code()` `.title()` 方法将直接返回 `String` 代替 `Box::leak` 获取到 `&'static str`,错误理解了该方法,导致忽略了其将引起内存泄漏。

## [0.5.6] - 2022-09-29

### 增加
Expand Down
6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "visdom"
version = "0.5.6"
version = "0.5.7"
edition = "2018"
description = "A html document syntax and operation library, use APIs similar to jquery, easy to use for web scraping and confused html."
keywords = ["html", "scrape", "jquery", "query", "selector"]
Expand All @@ -16,11 +16,11 @@ exclude = [".vscode/*.*", ".editorconfig", ".travis.yml", "src/main.rs", "perfor
rphtml = "0.5.6"
lazy_static = "1.4.0"
thiserror = "1.0.24"
regex = "1.4.3"
regex = "1.7.0"

[dev-dependencies]
crossbeam = "0.8.0"
criterion = "0.3.3"
criterion = "0.4.0"

[features]
default = []
Expand Down
8 changes: 2 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,6 @@ impl Dom {
}
}

fn to_static_str(orig: String) -> &'static str {
Box::leak(orig.into_boxed_str())
}

fn reset_next_siblings_index(start_index: usize, childs: &[RefNode]) {
for (step, node) in childs.iter().enumerate() {
node.borrow_mut().index = start_index + step;
Expand Down Expand Up @@ -1069,8 +1065,8 @@ impl IDocumentTrait for Document {
None
}
// source code
fn source_code(&self) -> &'static str {
to_static_str(self.doc.render(&Default::default()))
fn source_code(&self) -> String {
self.doc.render(&Default::default())
}
// get root node, in rphtml is abstract root node
fn get_root_node<'b>(&self) -> BoxDynNode<'b> {
Expand Down
7 changes: 3 additions & 4 deletions src/mesdoc/interface/document.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use super::{BoxDynElement, BoxDynNode, Elements};
use crate::mesdoc::error::BoxDynError;
use crate::mesdoc::utils::to_static_str;
use std::rc::Rc;

pub type MaybeDoc<'a> = Option<Box<dyn IDocumentTrait + 'a>>;
pub type IErrorHandle = Box<dyn Fn(BoxDynError)>;
pub trait IDocumentTrait {
fn get_element_by_id<'b>(&self, id: &str) -> Option<BoxDynElement<'b>>;
fn source_code(&self) -> &'static str;
fn source_code(&self) -> String;
// get root node
fn get_root_node<'b>(&self) -> BoxDynNode<'b>;
// document element, html tag
Expand All @@ -19,12 +18,12 @@ pub trait IDocumentTrait {
None
}
// title
fn title(&self) -> Option<&'static str> {
fn title(&self) -> Option<String> {
if let Some(root) = &self.get_root_node().root_element() {
let root = Elements::with_node(root);
let title = root.find("head").eq(0).find("title");
if !title.is_empty() {
return Some(to_static_str(String::from(title.text())));
return Some(title.text());
}
}
None
Expand Down
8 changes: 4 additions & 4 deletions src/mesdoc/interface/elements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::mesdoc::{
};
use crate::mesdoc::{
selector::rule::MatchSpecifiedHandle,
utils::{get_class_list, retain_by_index, to_static_str},
utils::{get_class_list, retain_by_index},
};
use std::collections::HashMap;
use std::collections::HashSet;
Expand Down Expand Up @@ -344,7 +344,7 @@ impl<'a> Elements<'a> {
/// let root = Vis::load(html)?;
/// let mut document = root.document();
/// assert!(document.is_some());
/// assert_eq!(document.unwrap().title(), Some("document"));
/// assert_eq!(document.unwrap().title(), Some(String::from("document")));
/// Ok(())
/// }
/// ```
Expand Down Expand Up @@ -2890,12 +2890,12 @@ impl<'a> Elements<'a> {
/// Ok(())
/// }
/// ```
pub fn text(&self) -> &str {
pub fn text(&self) -> String {
let mut result = String::with_capacity(50);
for ele in self.get_ref() {
result.push_str(&ele.text_content());
}
to_static_str(result)
result
}

/// Set the Elements's text, the html entity in content will auto encoded.
Expand Down
18 changes: 10 additions & 8 deletions src/mesdoc/rules/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,39 +11,41 @@ pub fn init(rules: &mut Vec<RuleItem>) {
r##"[{spaces}{attr_key}{spaces}{regexp#(?:([*^$~|!]?)=\s*(?:'((?:\\?+.)*?)'|([^\s\]'"<>/=`]+)|"((?:\\?+.)*?)"))?#}{spaces}]"##,
PRIORITY_ATTR_SELECTOR,
Box::new(|data: MatchedQueue| {
let def_mode = String::from("");
let attr_key = data[2].chars.iter().collect::<String>();
let value_data = &data[4].data;
let attr_value = value_data
.get("2")
.or_else(|| value_data.get("3"))
.or_else(|| value_data.get("4"))
.copied();
let match_mode = value_data.get("1").copied().unwrap_or("");
.map(|s| s.clone());
let match_mode = value_data.get("1").unwrap_or(&def_mode);
let handle: Box<dyn Fn(&Option<IAttrValue>) -> bool> = if let Some(attr_value) = attr_value {
let match_mode = match_mode.as_str();
if attr_value.is_empty() && !matches!(match_mode, "" | "!" | "|") {
// empty attribute value, ^$*
Box::new(|_val: &Option<IAttrValue>| false)
} else {
match match_mode {
// begin with value
"^" => Box::new(move |val: &Option<IAttrValue>| match val {
Some(IAttrValue::Value(v, _)) => v.starts_with(attr_value),
Some(IAttrValue::Value(v, _)) => v.starts_with(&attr_value),
_ => false,
}),
// end with value
"$" => Box::new(move |val: &Option<IAttrValue>| match val {
Some(IAttrValue::Value(v, _)) => v.ends_with(attr_value),
Some(IAttrValue::Value(v, _)) => v.ends_with(&attr_value),
_ => false,
}),
// contains value
"*" => Box::new(move |val: &Option<IAttrValue>| match val {
Some(IAttrValue::Value(v, _)) => v.contains(attr_value),
Some(IAttrValue::Value(v, _)) => v.contains(&attr_value),
_ => false,
}),
// either equal to value or start with `value` and followed `-`
"|" => Box::new(move |val: &Option<IAttrValue>| match val {
Some(IAttrValue::Value(v, _)) => {
if v == attr_value {
if *v == attr_value {
return true;
}
let attr_value: String = format!("{}-", attr_value);
Expand All @@ -66,12 +68,12 @@ pub fn init(rules: &mut Vec<RuleItem>) {
}),
// has a attribute and who's value not equal to setted value
"!" => Box::new(move |val: &Option<IAttrValue>| match val {
Some(IAttrValue::Value(v, _)) => attr_value != v,
Some(IAttrValue::Value(v, _)) => attr_value != *v,
_ => !attr_value.is_empty(),
}),
// equal to value
_ => Box::new(move |val: &Option<IAttrValue>| match val {
Some(IAttrValue::Value(v, _)) => v == attr_value,
Some(IAttrValue::Value(v, _)) => *v == attr_value,
_ => attr_value.is_empty(),
}),
}
Expand Down
11 changes: 6 additions & 5 deletions src/mesdoc/rules/pseudo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ use std::{cmp::Ordering, collections::VecDeque};
use std::{collections::HashMap, ops::Range};
const PRIORITY: u32 = PRIORITY_PSEUDO_SELECTOR;

fn nth_index_to_number(index: &Option<&str>) -> isize {
fn nth_index_to_number(index: &Option<String>) -> isize {
index
.as_ref()
.expect("Nth's n and index must have one")
.parse::<isize>()
.expect("Nth's index is not ok")
Expand Down Expand Up @@ -341,8 +342,8 @@ fn make_asc_or_desc_nth_child(selector: &'static str, asc: bool) -> RuleDefItem
PRIORITY,
Box::new(move |data: MatchedQueue| {
let nth_data = &data[2].data;
let n = nth_data.get("n").copied();
let index = nth_data.get("index").copied();
let n = nth_data.get("n").map(|s| s.clone());
let index = nth_data.get("index").map(|s| s.clone());
let handle = make_asc_or_desc_nth_child_handle(asc);
let specified_handle = if n.is_none() {
let index = nth_index_to_number(&index);
Expand Down Expand Up @@ -604,8 +605,8 @@ fn make_asc_or_desc_nth_of_type(selector: &'static str, asc: bool) -> RuleDefIte
PRIORITY,
Box::new(move |mut data: MatchedQueue| {
let nth_data = data.remove(2).data;
let n = nth_data.get("n").copied();
let index = nth_data.get("index").copied();
let n = nth_data.get("n").map(|s| s.clone());
let index = nth_data.get("index").map(|s| s.clone());
let specified_handle = if n.is_none() {
let index = nth_index_to_number(&index);
Some(make_asc_or_desc_nth_of_type_specified(asc, index))
Expand Down
Loading

0 comments on commit 9a7245e

Please sign in to comment.