Browse Source

WindowServer: Split double-duty Window::normalize_rect()

This commit:
- merges the two(!) places that defined independently the minimum size of a window.
- splits Window::normalize_rect(), which was originally just a function to apply
  the minimum size requirement, and has taken on the additional job of nudging
  windows back onto the desktop.

This inadvertantly fixes a crash that happens when a malicious program creates a
window of size (0, 0). Now, a window at [0,0 50x50] is created instead.
Ben Wiederhake 4 years ago
parent
commit
cf586311a6

+ 6 - 3
Userland/Services/WindowServer/ClientConnection.cpp

@@ -394,8 +394,10 @@ OwnPtr<Messages::WindowServer::SetWindowRectResponse> ClientConnection::handle(c
     if (message.rect().location() != window.rect().location()) {
         window.set_default_positioned(false);
     }
-    window.set_rect(message.rect());
-    window.normalize_rect();
+    auto rect = message.rect();
+    window.apply_minimum_size(rect);
+    window.set_rect(rect);
+    window.nudge_into_desktop();
     window.request_update(window.rect());
     return make<Messages::WindowServer::SetWindowRectResponse>(window.rect());
 }
@@ -452,8 +454,9 @@ OwnPtr<Messages::WindowServer::CreateWindowResponse> ClientConnection::handle(co
             rect = { WindowManager::the().get_recommended_window_position({ 100, 100 }), message.rect().size() };
             window->set_default_positioned(true);
         }
+        window->apply_minimum_size(rect);
         window->set_rect(rect);
-        window->normalize_rect();
+        window->nudge_into_desktop();
     }
     if (window->type() == WindowType::Desktop) {
         window->set_rect(WindowManager::the().desktop_rect());

+ 18 - 15
Userland/Services/WindowServer/Window.cpp

@@ -176,27 +176,30 @@ void Window::set_rect_without_repaint(const Gfx::IntRect& rect)
     m_frame.notify_window_rect_changed(old_rect, rect); // recomputes occlusions
 }
 
-void Window::normalize_rect(bool force_titlebar_visible)
+void Window::apply_minimum_size(Gfx::IntRect& rect)
+{
+    Gfx::IntSize minimum_size { 1, 1 };
+    if (type() == WindowType::Normal)
+        minimum_size = { 50, 50 };
+
+    rect.set_width(max(minimum_size.width(), rect.width()));
+    rect.set_height(max(minimum_size.height(), rect.height()));
+}
+
+void Window::nudge_into_desktop(bool force_titlebar_visible)
 {
     Gfx::IntRect arena = WindowManager::the().arena_rect_for_type(type());
-    auto min_size = 1;
     auto min_visible = 1;
-    if (type() == WindowType::Normal) {
-        min_size = 50;
+    if (type() == WindowType::Normal)
         min_visible = 30;
-    }
-
-    // Blow up to the appropriate size.
-    auto new_width = max(min_size, width());
-    auto new_height = max(min_size, height());
 
     // Push the frame around such that at least `min_visible` pixels of the *frame* are in the desktop rect.
     auto old_frame_rect = frame().rect();
     Gfx::IntRect new_frame_rect = {
-        clamp(old_frame_rect.x(), arena.left() + min_visible - new_width, arena.right() - min_visible),
-        clamp(old_frame_rect.y(), arena.top() + min_visible - new_height, arena.bottom() - min_visible),
-        old_frame_rect.width() + new_width - width(),
-        old_frame_rect.height() + new_height - height(),
+        clamp(old_frame_rect.x(), arena.left() + min_visible - width(), arena.right() - min_visible),
+        clamp(old_frame_rect.y(), arena.top() + min_visible - height(), arena.bottom() - min_visible),
+        old_frame_rect.width(),
+        old_frame_rect.height(),
     };
 
     // Make sure that at least half of the titlebar is visible.
@@ -209,8 +212,8 @@ void Window::normalize_rect(bool force_titlebar_visible)
     Gfx::IntRect new_window_rect = {
         x() + new_frame_rect.x() - old_frame_rect.x(),
         y() + new_frame_rect.y() - old_frame_rect.y(),
-        width() + new_frame_rect.width() - old_frame_rect.width(),
-        height() + new_frame_rect.height() - old_frame_rect.height(),
+        width(),
+        height(),
     };
     set_rect(new_window_rect);
 }

+ 2 - 1
Userland/Services/WindowServer/Window.h

@@ -159,7 +159,8 @@ public:
     void set_rect(const Gfx::IntRect&);
     void set_rect(int x, int y, int width, int height) { set_rect({ x, y, width, height }); }
     void set_rect_without_repaint(const Gfx::IntRect&);
-    void normalize_rect(bool force_titlebar_visible = true);
+    void apply_minimum_size(Gfx::IntRect&);
+    void nudge_into_desktop(bool force_titlebar_visible = true);
 
     void set_taskbar_rect(const Gfx::IntRect&);
     const Gfx::IntRect& taskbar_rect() const { return m_taskbar_rect; }

+ 4 - 5
Userland/Services/WindowServer/WindowManager.cpp

@@ -557,7 +557,7 @@ bool WindowManager::process_ongoing_window_move(MouseEvent& event, Window*& hove
                 // "Bounce back" the window if it would end up too far outside the screen.
                 // If the user has let go of Mod_Logo, maybe they didn't intentionally press it to begin with. Therefore, refuse to go into a state where knowledge about super-drags is necessary.
                 bool force_titlebar_visible = !(m_keyboard_modifiers & Mod_Logo);
-                m_move_window->normalize_rect(force_titlebar_visible);
+                m_move_window->nudge_into_desktop(force_titlebar_visible);
             } else if (pixels_moved_from_start > 5) {
                 m_move_window->set_untiled(event.position());
                 m_move_origin = event.position();
@@ -633,10 +633,9 @@ bool WindowManager::process_ongoing_window_resize(const MouseEvent& event, Windo
     auto new_rect = m_resize_window_original_rect;
 
     // First, size the new rect.
-    Gfx::IntSize minimum_size { 50, 50 };
-
-    new_rect.set_width(max(minimum_size.width(), new_rect.width() + change_w));
-    new_rect.set_height(max(minimum_size.height(), new_rect.height() + change_h));
+    new_rect.set_width(new_rect.width() + change_w);
+    new_rect.set_height(new_rect.height() + change_h);
+    m_resize_window->apply_minimum_size(new_rect);
 
     if (!m_resize_window->size_increment().is_null()) {
         int horizontal_incs = (new_rect.width() - m_resize_window->base_size().width()) / m_resize_window->size_increment().width();