From 2d5eac386f867975d47d112caf6fd184cc32c19f Mon Sep 17 00:00:00 2001
From: Integral <integral@member.fsf.org>
Date: Sun, 14 Sep 2025 10:37:50 +0800
Subject: [PATCH] servoshell: Add `ParseResolutionError` to parse elegantly
 (#39289)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When passing an invalid resolution string (such as `1x1x1`) to the
`--screen-size` or `--window-size` argument, Servo starts without any
error. Additionally, if the width or height is set to 0, Servo crashes
with a SIGSEGV (Address boundary error).

This patch addresses the following issue by several changes:

1. Introduce a custom error type ParseResolutionError.
2. Replace the `split()` method with `split_once()`.
3. Make the capital 'X' an acceptable separator.
4. Add a check to prevent crashes when width or height is set to 0.

---

Before:
```
╰─❯ ./servo --screen-size=0
index out of bounds: the len is 1 but the index is 1 (thread main, at ports/servoshell/prefs.rs:236)
fish: Job 1, './servo --screen-size=0' terminated by signal SIGSEGV (Address boundary error)
```
```
╰─❯ ./servo --screen-size=0x1
xdg_surface#30: error -1: invalid window geometry size (0x1)
assertion `left != right` failed
  left: 0
 right: 0 (thread main, at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/surfman-0.10.0/src/platform/unix/wayland/connection.rs:140)
fish: Job 1, './servo --screen-size=0x1' terminated by signal SIGSEGV (Address boundary error)
```
After:
```
╰─❯ ./servo --screen-size=0
Error: couldn't parse `0`: invalid resolution format
```
```
╰─❯ ./servo --screen-size=0x1
Error: couldn't parse `0x1`: width and height must be greater than 0
```

Signed-off-by: Integral <integral@member.fsf.org>
---
 ports/servoshell/prefs.rs | 46 ++++++++++++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/ports/servoshell/prefs.rs b/ports/servoshell/prefs.rs
index 737ef4d8dfa..29bb978c48c 100644
--- a/ports/servoshell/prefs.rs
+++ b/ports/servoshell/prefs.rs
@@ -10,7 +10,7 @@ use std::path::{Path, PathBuf};
 use std::str::FromStr;
 #[cfg(any(target_os = "android", target_env = "ohos"))]
 use std::sync::OnceLock;
-use std::{env, process};
+use std::{env, fmt, process};
 
 use bpaf::*;
 use euclid::Size2D;
@@ -221,18 +221,48 @@ pub(crate) enum ArgumentParsingResult {
     ErrorParsing,
 }
 
+enum ParseResolutionError {
+    InvalidFormat,
+    ZeroDimension,
+    ParseError(std::num::ParseIntError),
+}
+
+impl fmt::Display for ParseResolutionError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            ParseResolutionError::InvalidFormat => write!(f, "invalid resolution format"),
+            ParseResolutionError::ZeroDimension => {
+                write!(f, "width and height must be greater than 0")
+            },
+            ParseResolutionError::ParseError(e) => write!(f, "{e}"),
+        }
+    }
+}
+
 /// Parse a resolution string into a Size2D.
 fn parse_resolution_string(
     string: String,
-) -> Result<Option<Size2D<u32, DeviceIndependentPixel>>, std::num::ParseIntError> {
+) -> Result<Option<Size2D<u32, DeviceIndependentPixel>>, ParseResolutionError> {
     if string.is_empty() {
         Ok(None)
     } else {
-        let components = string
-            .split('x')
-            .map(|component| component.parse::<u32>())
-            .collect::<Result<Vec<_>, std::num::ParseIntError>>()?;
-        Ok(Some(Size2D::new(components[0], components[1])))
+        let (width, height) = string
+            .split_once(['x', 'X'])
+            .ok_or(ParseResolutionError::InvalidFormat)?;
+
+        let width = width.trim();
+        let height = height.trim();
+        if width.is_empty() || height.is_empty() {
+            return Err(ParseResolutionError::InvalidFormat);
+        }
+
+        let width = width.parse().map_err(ParseResolutionError::ParseError)?;
+        let height = height.parse().map_err(ParseResolutionError::ParseError)?;
+        if width == 0 || height == 0 {
+            return Err(ParseResolutionError::ZeroDimension);
+        }
+
+        Ok(Some(Size2D::new(width, height)))
     }
 }
 
@@ -269,7 +299,7 @@ fn flag_with_default_parser<S, T>(
 ) -> impl Parser<Option<T>>
 where
     S: FromStr + 'static,
-    <S as FromStr>::Err: std::fmt::Display,
+    <S as FromStr>::Err: fmt::Display,
     T: Clone + 'static,
 {
     let just_flag = if let Some(c) = short_cmd {