commit daeba37: [Rework] Html/CSS: Remove css C bindings as they are useless now
Vsevolod Stakhov
vsevolod at highsecure.ru
Tue Jun 15 14:28:04 UTC 2021
Author: Vsevolod Stakhov
Date: 2021-06-15 12:54:47 +0100
URL: https://github.com/rspamd/rspamd/commit/daeba37c5557e5c33d6e752a204b723e657532a4
[Rework] Html/CSS: Remove css C bindings as they are useless now
---
src/libserver/css/css.cxx | 56 +++++++++++----------------------------
src/libserver/css/css.h | 45 -------------------------------
src/libserver/css/css.hxx | 7 ++++-
src/libserver/css/css_parser.cxx | 36 +++++++++----------------
src/libserver/css/css_parser.hxx | 4 +--
src/libserver/css/parse_error.hxx | 4 +--
src/libserver/html/html.cxx | 22 ++++++++-------
src/libserver/html/html.hxx | 7 ++++-
8 files changed, 58 insertions(+), 123 deletions(-)
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<rspamd::css::css_style_sheet *>(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<rspamd::css::css_style_sheet*>(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<rspamd_css_ptr>(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<int>(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<css_selector> &&selector,
}
}
+auto
+css_parse_style(rspamd_mempool_t *pool,
+ std::string_view input,
+ std::shared_ptr<css_style_sheet> &&existing)
+ -> css_return_pair
+{
+ auto parse_res = rspamd::css::parse_css(pool, input,
+ std::forward<std::shared_ptr<css_style_sheet>>(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 <string>
#include <memory>
#include "logger.h"
-#include "css.h"
#include "css_rule.hxx"
#include "css_selector.hxx"
@@ -50,6 +49,12 @@ private:
std::unique_ptr<impl> pimpl;
};
+using css_return_pair = std::pair<std::shared_ptr<css_style_sheet>, css_parse_error>;
+auto css_parse_style(rspamd_mempool_t *pool,
+ std::string_view input,
+ std::shared_ptr<css_style_sheet> &&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<css_style_sheet> &&existing, rspamd_mempool_t *pool) :
style_object(existing), pool(pool) {}
/*
@@ -169,9 +168,9 @@ public:
std::unique_ptr<css_consumed_block> consume_css_rule(const std::string_view &sv);
bool consume_input(const std::string_view &sv);
- auto get_object_maybe(void) -> tl::expected<std::unique_ptr<css_style_sheet>, css_parse_error> {
+ auto get_object_maybe(void) -> tl::expected<std::shared_ptr<css_style_sheet>, 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<css_style_sheet> style_object;
+ std::shared_ptr<css_style_sheet> style_object;
std::unique_ptr<css_tokeniser> 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<css_consumed_block> &top) -> bool;
@@ -618,7 +609,7 @@ bool css_parser::consume_input(const std::string_view &sv)
}
if (!style_object) {
- style_object = std::make_unique<css_style_sheet>(pool);
+ style_object = std::make_shared<css_style_sheet>(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<std::unique_ptr<css_style_sheet>, css_parse_error>
+ std::shared_ptr<css_style_sheet> &&other)
+ -> tl::expected<std::shared_ptr<css_style_sheet>, css_parse_error>
{
- css_parser parser(other, pool);
+ css_parser parser(std::forward<std::shared_ptr<css_style_sheet>>(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<css_style_sheet> 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<std::unique_ptr<css_style_sheet>, css_parse_error>;
+ std::shared_ptr<css_style_sheet> &&other)
+ -> tl::expected<std::shared_ptr<css_style_sheet>, 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 </s substring and then pass
* the content to the parser (if needed)
*/
- goffset end_style = rspamd_substring_search (p, end - p,
+ auto end_style = rspamd_substring_search (p, end - p,
"</", 2);
if (end_style == -1 || g_ascii_tolower (p[end_style + 2]) != 's') {
/* Invalid style */
@@ -1390,14 +1390,18 @@ html_process_input(rspamd_mempool_t *pool,
else {
if (allow_css) {
- GError *err = nullptr;
- hc->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<int>(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 <memory>
#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<html_image *> images;
std::vector<std::unique_ptr<struct html_tag>> all_tags;
std::string parsed;
- void *css_style;
+ std::shared_ptr<css::css_style_sheet> css_style;
/* Preallocate and reserve all internal structures */
html_content() {
More information about the Commits
mailing list