Ladybird/Qt: Sanitize user-provided URLs with LibWebView

Author: https://github.com/trflynn89
Commit: https://github.com/SerenityOS/serenity/commit/f023e37de7
Pull-request: https://github.com/SerenityOS/serenity/pull/21435
Reviewed-by: https://github.com/ADKaster ✅

Ladybird/CMakeLists.txt

@@ -174,7 +174,7 @@ target_sources(ladybird PUBLIC FILE_SET ladybird TYPE HEADERS
     BASE_DIRS ${SERENITY_SOURCE_DIR}
     FILES ${LADYBIRD_HEADERS}
 )
-target_link_libraries(ladybird PRIVATE LibCore LibFileSystem LibGfx LibGUI LibIPC LibJS LibMain LibPublicSuffix LibWeb LibWebView LibProtocol)
+target_link_libraries(ladybird PRIVATE LibCore LibFileSystem LibGfx LibGUI LibIPC LibJS LibMain LibWeb LibWebView LibProtocol)
 
 target_include_directories(ladybird PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
 target_include_directories(ladybird PRIVATE ${SERENITY_SOURCE_DIR}/Userland/)

Ladybird/Qt/AutoComplete.cpp

@@ -121,12 +121,6 @@ ErrorOr<void> AutoComplete::got_network_response(QNetworkReply* reply)
     return Error::from_string_view("Invalid engine name"sv);
 }
 
-ErrorOr<String> AutoComplete::search_url_from_query(StringView query)
-{
-    auto search_engine = TRY(ak_string_from_qstring(Settings::the()->search_engine().url));
-    return search_engine.replace("{}"sv, AK::URL::percent_encode(query), ReplaceMode::FirstOnly);
-}
-
 ErrorOr<String> AutoComplete::auto_complete_url_from_query(StringView query)
 {
     auto autocomplete_engine = TRY(ak_string_from_qstring(Settings::the()->autocomplete_engine().url));

Ladybird/Qt/AutoComplete.h

@@ -61,7 +61,6 @@ public:
 
     ErrorOr<void> get_search_suggestions(StringView);
     void clear_suggestions();
-    static ErrorOr<String> search_url_from_query(StringView query);
     static ErrorOr<String> auto_complete_url_from_query(StringView query);
 
 signals:

Ladybird/Qt/BrowserWindow.cpp

@@ -423,15 +423,9 @@ BrowserWindow::BrowserWindow(Vector<URL> const& initial_urls, WebView::CookieJar
         m_tabs_container->setCurrentIndex(m_tabs_container->count() - 1);
     });
 
-    if (!initial_urls.is_empty()) {
-        bool is_first_tab = true;
-        for (auto const& url : initial_urls) {
-            auto initial_url_string = qstring_from_ak_deprecated_string(url.serialize());
-            new_tab(initial_url_string, is_first_tab ? Web::HTML::ActivateTab::Yes : Web::HTML::ActivateTab::No);
-            is_first_tab = false;
-        }
-    } else {
-        new_tab(Settings::the()->new_tab_page(), Web::HTML::ActivateTab::Yes);
+    for (size_t i = 0; i < initial_urls.size(); ++i) {
+        auto url_string = qstring_from_ak_deprecated_string(initial_urls[i].serialize());
+        new_tab(url_string, (i == 0) ? Web::HTML::ActivateTab::Yes : Web::HTML::ActivateTab::No);
     }
 
     setCentralWidget(m_tabs_container);

Ladybird/Qt/LocationEdit.cpp

@@ -8,7 +8,7 @@
 #include "Settings.h"
 #include "StringUtils.h"
 #include <AK/URL.h>
-#include <LibPublicSuffix/URL.h>
+#include <LibWebView/URL.h>
 #include <QApplication>
 #include <QPalette>
 #include <QTextLayout>
@@ -29,21 +29,17 @@ LocationEdit::LocationEdit(QWidget* parent)
 
     connect(this, &QLineEdit::returnPressed, [&] {
         clearFocus();
-        if (!Settings::the()->enable_search())
-            return;
 
-        auto query = ak_deprecated_string_from_qstring(text());
-        if (auto result = PublicSuffix::absolute_url(query); !result.is_error())
-            return;
-
-        auto search_url_or_error = AutoComplete::search_url_from_query(query);
-        if (search_url_or_error.is_error()) {
-            dbgln("LocationEdit::returnPressed: search_url_from_query failed: {}", search_url_or_error.error());
-            return;
+        Optional<String> search_engine_url;
+        if (Settings::the()->enable_search()) {
+            auto search_engine = Settings::the()->search_engine();
+            search_engine_url = MUST(ak_string_from_qstring(search_engine.url));
         }
-        auto search_url = search_url_or_error.release_value();
 
-        setText(qstring_from_ak_string(search_url));
+        auto query = MUST(ak_string_from_qstring(text()));
+
+        if (auto url = WebView::sanitize_url(query, search_engine_url.map([](auto& value) { return value.bytes_as_string_view(); })); url.has_value())
+            setText(qstring_from_ak_deprecated_string(url->serialize()));
     });
 
     connect(this, &QLineEdit::textEdited, [this] {

Ladybird/Qt/Tab.cpp

@@ -14,6 +14,7 @@
 #include <LibGfx/ImageFormats/BMPWriter.h>
 #include <LibGfx/Painter.h>
 #include <LibWebView/SourceHighlighter.h>
+#include <LibWebView/URL.h>
 #include <QClipboard>
 #include <QColorDialog>
 #include <QCoreApplication>
@@ -556,13 +557,9 @@ void Tab::focus_location_editor()
     m_location_edit->selectAll();
 }
 
-void Tab::navigate(QString url_qstring)
+void Tab::navigate(QString const& url_qstring)
 {
-    auto url_string = ak_deprecated_string_from_qstring(url_qstring);
-    if (url_string.starts_with('/'))
-        url_string = DeprecatedString::formatted("file://{}", url_string);
-    else if (URL url = url_string; !url.is_valid())
-        url_string = DeprecatedString::formatted("https://{}", url_string);
+    auto url_string = MUST(ak_string_from_qstring(url_qstring));
     view().load(url_string);
 }
 
@@ -625,7 +622,7 @@ void Tab::open_file()
 {
     auto filename = QFileDialog::getOpenFileName(this, "Open file", QDir::homePath(), "All Files (*.*)");
     if (!filename.isNull())
-        navigate("file://" + filename);
+        navigate(filename);
 }
 
 int Tab::tab_index()

Ladybird/Qt/Tab.h

@@ -33,7 +33,7 @@ public:
 
     WebContentView& view() { return *m_view; }
 
-    void navigate(QString);
+    void navigate(QString const&);
     void load_html(StringView);
 
     void back();

Ladybird/Qt/main.cpp

@@ -15,11 +15,11 @@
 #include <LibCore/EventLoop.h>
 #include <LibCore/Process.h>
 #include <LibCore/System.h>
-#include <LibFileSystem/FileSystem.h>
 #include <LibGfx/Font/FontDatabase.h>
 #include <LibMain/Main.h>
 #include <LibWebView/CookieJar.h>
 #include <LibWebView/Database.h>
+#include <LibWebView/URL.h>
 #include <QApplication>
 
 namespace Ladybird {
@@ -85,15 +85,6 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     args_parser.add_option(use_lagom_networking, "Enable Lagom servers for networking", "enable-lagom-networking", 0);
     args_parser.parse(arguments);
 
-    auto get_formatted_url = [&](StringView const& raw_url) -> ErrorOr<URL> {
-        URL url = raw_url;
-        if (FileSystem::exists(raw_url))
-            url = URL::create_with_file_scheme(TRY(FileSystem::real_path(raw_url)).to_deprecated_string());
-        else if (!url.is_valid())
-            url = DeprecatedString::formatted("https://{}", raw_url);
-        return url;
-    };
-
     RefPtr<WebView::Database> database;
 
     if (enable_sql_database) {
@@ -106,8 +97,13 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     Vector<URL> initial_urls;
 
     for (auto const& raw_url : raw_urls) {
-        if (auto url = TRY(get_formatted_url(raw_url)); url.is_valid())
-            initial_urls.append(move(url));
+        if (auto url = WebView::sanitize_url(raw_url); url.has_value())
+            initial_urls.append(url.release_value());
+    }
+
+    if (initial_urls.is_empty()) {
+        auto new_tab_page = Ladybird::Settings::the()->new_tab_page();
+        initial_urls.append(MUST(ak_string_from_qstring(new_tab_page)));
     }
 
     Ladybird::BrowserWindow window(initial_urls, cookie_jar, webdriver_content_ipc_path, enable_callgrind_profiling ? WebView::EnableCallgrindProfiling::Yes : WebView::EnableCallgrindProfiling::No, use_lagom_networking ? Ladybird::UseLagomNetworking::Yes : Ladybird::UseLagomNetworking::No);