Skip to content

Commit

Permalink
Fix lol_html_take_last_error() after calling lol_html_element_add_end…
Browse files Browse the repository at this point in the history
…_tag_handler() on element with no end tag (#187)

* Remove unused "No end tag" error

This was removed when the on_end_tag() API was switched to end_tag_handlers().

* Make lol_html_element_add_end_tag_handler() arm lol_html_take_last_error() on no end tag

Prior to the end_tag_handlers() refactor, calling lol_html_element_on_end_tag() on an element with no end tag would arm lol_html_take_last_error() with a "No end tag." string. The new lol_html_element_add_end_tag_handler() function inherited the previous function's doc comment describing this behavior, but not the actual behavior itself. This commit restores that behavior and adds a regression test.
  • Loading branch information
harrishancock authored Jun 7, 2023
1 parent a005329 commit 952220c
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 21 deletions.
4 changes: 4 additions & 0 deletions c-api/include/lol_html.h
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,10 @@ void *lol_html_element_user_data_get(const lol_html_element_t *element);
// stops immediately and `write()` or `end()` of the rewriter methods
// return an error code.
//
// Not all elements (for example, `<br>`) support end tags. If this function is
// called on such an element, this function returns an error code as described
// below.
//
// Returns 0 in case of success and -1 otherwise. The actual error message
// can be obtained using `lol_html_take_last_error` function.
//
Expand Down
22 changes: 11 additions & 11 deletions c-api/src/element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,18 +228,18 @@ pub extern "C" fn lol_html_element_add_end_tag_handler(
) -> c_int {
let element = to_ref_mut!(element);

match element.end_tag_handlers() {
Some(handlers) => {
handlers.push(Box::new(move |end_tag| {
match unsafe { handler(end_tag, user_data) } {
RewriterDirective::Continue => Ok(()),
RewriterDirective::Stop => Err("The rewriter has been stopped.".into()),
}
}));
0
let handlers = unwrap_or_ret_err_code! {
element.end_tag_handlers().ok_or("No end tag.")
};

handlers.push(Box::new(move |end_tag| {
match unsafe { handler(end_tag, user_data) } {
RewriterDirective::Continue => Ok(()),
RewriterDirective::Stop => Err("The rewriter has been stopped.".into()),
}
None => -1,
}
}));

0
}

#[no_mangle]
Expand Down
56 changes: 56 additions & 0 deletions c-api/tests/src/test_element_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,36 @@ EXPECT_OUTPUT(
sizeof(EXPECTED_USER_DATA)
);

static lol_html_rewriter_directive_t noop_end_tag_handler(lol_html_end_tag_t *end_tag, void *user_data) {
UNUSED(end_tag);
UNUSED(user_data);
return LOL_HTML_CONTINUE;
}

static lol_html_rewriter_directive_t add_end_tag_handler_to_element_with_no_end_tag(
lol_html_element_t *element,
void *user_data
) {
UNUSED(user_data);

ok(lol_html_element_add_end_tag_handler(element, noop_end_tag_handler, NULL) == -1);

lol_html_str_t msg = lol_html_take_last_error();

str_eq(msg, "No end tag.");

lol_html_str_free(msg);

return LOL_HTML_CONTINUE;
}

EXPECT_OUTPUT(
no_end_tag,
"<br>",
&EXPECTED_USER_DATA,
sizeof(EXPECTED_USER_DATA)
);

void element_api_test() {
int user_data = 43;

Expand Down Expand Up @@ -787,4 +817,30 @@ void element_api_test() {
const char *input = "<div>42</div><div>some data</div>";
run_rewriter(builder, input, modify_element_end_tag, &user_data);
}

{
note("NoEndTag");

const char *selector_str = "br";
lol_html_selector_t *selector = lol_html_selector_parse(
selector_str,
strlen(selector_str)
);

lol_html_rewriter_builder_t *builder = lol_html_rewriter_builder_new();

lol_ok(lol_html_rewriter_builder_add_element_content_handlers(
builder,
selector,
add_end_tag_handler_to_element_with_no_end_tag,
NULL,
NULL,
NULL,
NULL,
NULL
));

const char *input = "<br>";
run_rewriter(builder, input, no_end_tag, &user_data);
}
}
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ pub mod errors {
/// HTML content descriptors that can be produced and modified by a rewriter.
pub mod html_content {
pub use super::rewritable_units::{
Attribute, Comment, ContentType, Doctype, DocumentEnd, Element, EndTag, EndTagError,
StartTag, TextChunk, UserData,
Attribute, Comment, ContentType, Doctype, DocumentEnd, Element, EndTag, StartTag,
TextChunk, UserData,
};

pub use super::html::TextType;
Expand Down
8 changes: 0 additions & 8 deletions src/rewritable_units/element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,6 @@ pub enum TagNameError {
UnencodableCharacter,
}

/// An error that occurs when invalid value is provided for the tag name.
#[derive(Error, Debug, Eq, PartialEq, Copy, Clone)]
pub enum EndTagError {
/// The tag has no end tag.
#[error("No end tag.")]
NoEndTag,
}

/// An HTML element rewritable unit.
///
/// Exposes API for examination and modification of a parsed HTML element.
Expand Down

0 comments on commit 952220c

Please sign in to comment.