From: Vsevolod Stakhov Date: Tue, 15 Jun 2021 11:54:47 +0000 (+0100) Subject: [Rework] Html/CSS: Remove css C bindings as they are useless now X-Git-Tag: 3.0~303 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=daeba37c5557e5c33d6e752a204b723e657532a4;p=rspamd.git [Rework] Html/CSS: Remove css C bindings as they are useless now --- diff --git a/src/libserver/css/css.cxx b/src/libserver/css/css.cxx index 9b0e02230..12f7753c7 100644 --- a/src/libserver/css/css.cxx +++ b/src/libserver/css/css.cxx @@ -14,7 +14,6 @@ * limitations under the License. */ -#include "css.h" #include "css.hxx" #include "contrib/robin-hood/robin_hood.h" #include "css_parser.hxx" @@ -23,45 +22,6 @@ #define DOCTEST_CONFIG_IMPLEMENT #include "doctest/doctest.h" -static void -rspamd_css_dtor(void *p) -{ - rspamd::css::css_style_sheet *style = - reinterpret_cast(p); - - delete style; -} - -rspamd_css_ptr -rspamd_css_parse_style(rspamd_mempool_t *pool, const gchar *begin, gsize len, - rspamd_css_ptr existing_style, - GError **err) -{ - auto parse_res = rspamd::css::parse_css(pool, {(const char* )begin, len}, - reinterpret_cast(existing_style)); - - if (parse_res.has_value()) { - /* - * Detach style pointer from the unique_ptr as it will be managed by - * C memory pool - */ - auto *detached_style = reinterpret_cast(parse_res.value().release()); - - /* We attach dtor merely if the existing style is not null */ - if (!existing_style) { - rspamd_mempool_add_destructor(pool, rspamd_css_dtor, (void *) detached_style); - } - - return detached_style; - } - else { - g_set_error(err, g_quark_from_static_string("css"), - static_cast(parse_res.error().type), - "parse error"); - return nullptr; - } -} - namespace rspamd::css { INIT_LOG_MODULE_PUBLIC(css); @@ -136,4 +96,20 @@ css_style_sheet::add_selector_rule(std::unique_ptr &&selector, } } +auto +css_parse_style(rspamd_mempool_t *pool, + std::string_view input, + std::shared_ptr &&existing) + -> css_return_pair +{ + auto parse_res = rspamd::css::parse_css(pool, input, + std::forward>(existing)); + + if (parse_res.has_value()) { + return std::make_pair(parse_res.value(), css_parse_error()); + } + + return std::make_pair(nullptr, parse_res.error()); +} + } \ No newline at end of file diff --git a/src/libserver/css/css.h b/src/libserver/css/css.h deleted file mode 100644 index 607f1fa2c..000000000 --- a/src/libserver/css/css.h +++ /dev/null @@ -1,45 +0,0 @@ -/*- - * Copyright 2021 Vsevolod Stakhov - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef RSPAMD_CSS_H -#define RSPAMD_CSS_H - -#include "config.h" -#include "mem_pool.h" - -#ifdef __cplusplus -extern "C" { -#endif -typedef void * rspamd_css_ptr; - -rspamd_css_ptr rspamd_css_parse_style (rspamd_mempool_t *pool, - const gchar *begin, - gsize len, - rspamd_css_ptr existing_style, - GError **err); - -/* - * Unescape css - */ -const gchar *rspamd_css_unescape (rspamd_mempool_t *pool, - const guchar *begin, - gsize len, - gsize *outlen); -#ifdef __cplusplus -} -#endif - -#endif //RSPAMD_CSS_H diff --git a/src/libserver/css/css.hxx b/src/libserver/css/css.hxx index 739ad3251..a169a1052 100644 --- a/src/libserver/css/css.hxx +++ b/src/libserver/css/css.hxx @@ -21,7 +21,6 @@ #include #include #include "logger.h" -#include "css.h" #include "css_rule.hxx" #include "css_selector.hxx" @@ -50,6 +49,12 @@ private: std::unique_ptr pimpl; }; +using css_return_pair = std::pair, css_parse_error>; +auto css_parse_style(rspamd_mempool_t *pool, + std::string_view input, + std::shared_ptr &&existing) -> + css_return_pair; + } #endif //RSPAMD_CSS_H \ No newline at end of file diff --git a/src/libserver/css/css_parser.cxx b/src/libserver/css/css_parser.cxx index d40520381..6526ebc57 100644 --- a/src/libserver/css/css_parser.cxx +++ b/src/libserver/css/css_parser.cxx @@ -148,15 +148,14 @@ class css_parser { public: css_parser(void) = delete; /* Require mempool to be set for logging */ explicit css_parser(rspamd_mempool_t *pool) : pool (pool) { - /* Int this case we need to remove style in case of parser error */ - owned_style = true; + style_object.reset(); } /* * This constructor captures existing via unique_ptr, but it does not * destruct it on errors (we assume that it is owned somewhere else) */ - explicit css_parser(css_style_sheet *existing, rspamd_mempool_t *pool) : + explicit css_parser(std::shared_ptr &&existing, rspamd_mempool_t *pool) : style_object(existing), pool(pool) {} /* @@ -169,9 +168,9 @@ public: std::unique_ptr consume_css_rule(const std::string_view &sv); bool consume_input(const std::string_view &sv); - auto get_object_maybe(void) -> tl::expected, css_parse_error> { + auto get_object_maybe(void) -> tl::expected, css_parse_error> { if (style_object) { - return std::move(style_object); + return style_object; } return tl::make_unexpected(error); @@ -180,15 +179,8 @@ public: /* Helper parser methods */ static bool need_unescape(const std::string_view &sv); - ~css_parser() { - if (!owned_style && style_object) { - /* Avoid double free */ - (void)style_object.release(); - } - } - private: - std::unique_ptr style_object; + std::shared_ptr style_object; std::unique_ptr tokeniser; css_parse_error error; @@ -197,7 +189,6 @@ private: int rec_level = 0; const int max_rec = 20; bool eof = false; - bool owned_style = false; /* Consumers */ auto component_value_consumer(std::unique_ptr &top) -> bool; @@ -618,7 +609,7 @@ bool css_parser::consume_input(const std::string_view &sv) } if (!style_object) { - style_object = std::make_unique(pool); + style_object = std::make_shared(pool); } for (auto &&rule : rules) { @@ -772,10 +763,10 @@ get_rules_parser_functor(rspamd_mempool_t *pool, * Wrapper for the parser */ auto parse_css(rspamd_mempool_t *pool, const std::string_view &st, - css_style_sheet *other) - -> tl::expected, css_parse_error> + std::shared_ptr &&other) + -> tl::expected, css_parse_error> { - css_parser parser(other, pool); + css_parser parser(std::forward>(other), pool); std::string_view processed_input; if (css_parser::need_unescape(st)) { @@ -878,14 +869,13 @@ TEST_SUITE("css parser") { } /* We now merge all styles together */ - css_style_sheet *merged = nullptr; + std::shared_ptr merged; for (const auto &c : cases) { - auto ret = parse_css(pool, c, merged); - /* Avoid destruction as we are using this to interoperate with C */ - merged = ret->release(); + auto ret = parse_css(pool, c, std::move(merged)); + merged.swap(ret.value()); } - CHECK(merged != nullptr); + CHECK(merged.get() != nullptr); rspamd_mempool_delete(pool); } diff --git a/src/libserver/css/css_parser.hxx b/src/libserver/css/css_parser.hxx index 1e0762d78..35b51ed23 100644 --- a/src/libserver/css/css_parser.hxx +++ b/src/libserver/css/css_parser.hxx @@ -193,8 +193,8 @@ class css_style_sheet; * Update the existing stylesheet with another stylesheet */ auto parse_css(rspamd_mempool_t *pool, const std::string_view &st, - css_style_sheet *other) - -> tl::expected, css_parse_error>; + std::shared_ptr &&other) + -> tl::expected, css_parse_error>; /* * Creates a functor to consume css selectors sequence diff --git a/src/libserver/css/parse_error.hxx b/src/libserver/css/parse_error.hxx index 458469afc..450c49d68 100644 --- a/src/libserver/css/parse_error.hxx +++ b/src/libserver/css/parse_error.hxx @@ -33,6 +33,7 @@ enum class css_parse_error_type { PARSE_ERROR_BAD_NESTING, PARSE_ERROR_NYI, PARSE_ERROR_UNKNOWN_ERROR, + PARSE_ERROR_NO_ERROR, }; struct css_parse_error { @@ -41,9 +42,8 @@ struct css_parse_error { explicit css_parse_error (css_parse_error_type type, const std::string &description) : type(type), description(description) {} - explicit css_parse_error (css_parse_error_type type) : + explicit css_parse_error (css_parse_error_type type = css_parse_error_type::PARSE_ERROR_NO_ERROR) : type(type) {} - css_parse_error() = default; }; } diff --git a/src/libserver/html/html.cxx b/src/libserver/html/html.cxx index f30b9d1b8..5c0753d4a 100644 --- a/src/libserver/html/html.cxx +++ b/src/libserver/html/html.cxx @@ -21,11 +21,11 @@ #include "html_block.hxx" #include "html.hxx" #include "libserver/css/css_value.hxx" +#include "libserver/css/css.hxx" #include "url.h" #include "contrib/libucl/khash.h" #include "libmime/images.h" -#include "css/css.h" #include "libutil/cxx/utf8_util.h" #include "html_tag_defs.hxx" @@ -1381,7 +1381,7 @@ html_process_input(rspamd_mempool_t *pool, * We just search for the first css_style = rspamd_css_parse_style(pool, p, end_style, hc->css_style, - &err); - - if (err) { - msg_info_pool ("cannot parse css: %e", err); - g_error_free (err); + auto ret_maybe = rspamd::css::parse_css(pool, {p, std::size_t(end_style)}, + std::move(hc->css_style)); + + if (!ret_maybe.has_value()) { + auto err_str = fmt::format("cannot parse css (error code: {}): {}", + static_cast(ret_maybe.error().type), + ret_maybe.error().description.value_or("unknown error")); + msg_info_pool ("cannot parse css: %*s", + (int)err_str.size(), err_str.data()); } + + hc->css_style = ret_maybe.value(); } p += end_style; diff --git a/src/libserver/html/html.hxx b/src/libserver/html/html.hxx index 3fd778ade..d4a2fb58b 100644 --- a/src/libserver/html/html.hxx +++ b/src/libserver/html/html.hxx @@ -29,6 +29,11 @@ #include #include "function2/function2.hpp" +namespace rspamd::css { +/* Forward declaration */ +class css_style_sheet; +} + namespace rspamd::html { struct html_block; @@ -42,7 +47,7 @@ struct html_content { std::vector images; std::vector> all_tags; std::string parsed; - void *css_style; + std::shared_ptr css_style; /* Preallocate and reserve all internal structures */ html_content() {