Prevent negative cell dimensions (#1181)

Prevent the cell dimensions from going below 1, this bug resulted in
allocation of large amounts of memory in the scrollback PR but is also
present on master.

Currently the approach is to just `panic!`, however an `eprintln!` and
`exit` could be an alternative too. I don't think it's realistic to
check this at startup and it should have no performance impact since the
failing method is only called once at startup.

To make it a bit more clear what kind of values are accepted, the
datatypes of offsets and paddings have also been changed so that these
don't accept floats anymore and padding can never be negative.

This should allow us to be a bit more strict with the config to make
sure that errors are printed when invalid values are specified (like
negative padding).

This fixes #1167.

3 files changed, 32 insertions, 32 deletions

diff --git a/src/config.rs b/src/config.rs
index 0eb7a0d7..f8e6490d 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -270,18 +270,18 @@ pub struct WindowConfig {
 
     /// Pixel padding
     #[serde(default="default_padding", deserialize_with = "deserialize_padding")]
-    padding: Delta,
+    padding: Delta<u8>,
 
     /// Draw the window with title bar / borders
     #[serde(default, deserialize_with = "failure_default")]
     decorations: bool,
 }
 
-fn default_padding() -> Delta {
-    Delta { x: 2., y: 2. }
+fn default_padding() -> Delta<u8> {
+    Delta { x: 2, y: 2 }
 }
 
-fn deserialize_padding<'a, D>(deserializer: D) -> ::std::result::Result<Delta, D::Error>
+fn deserialize_padding<'a, D>(deserializer: D) -> ::std::result::Result<Delta<u8>, D::Error>
     where D: de::Deserializer<'a>
 {
     match Delta::deserialize(deserializer) {
@@ -318,7 +318,7 @@ pub struct Config {
 
     /// Pixel padding
     #[serde(default, deserialize_with = "failure_default")]
-    padding: Option<Delta>,
+    padding: Option<Delta<u8>>,
 
     /// TERM env variable
     #[serde(default, deserialize_with = "failure_default")]
@@ -1285,7 +1285,7 @@ impl Config {
         self.tabspaces
     }
 
-    pub fn padding(&self) -> &Delta {
+    pub fn padding(&self) -> &Delta<u8> {
         self.padding.as_ref()
             .unwrap_or(&self.window.padding)
     }
@@ -1448,20 +1448,15 @@ impl Dimensions {
 }
 
 /// A delta for a point in a 2 dimensional plane
-#[derive(Clone, Copy, Debug, Deserialize)]
-pub struct Delta {
+#[derive(Clone, Copy, Debug, Default, Deserialize)]
+#[serde(bound(deserialize = "T: Deserialize<'de> + Default"))]
+pub struct Delta<T: Default> {
     /// Horizontal change
     #[serde(default, deserialize_with = "failure_default")]
-    pub x: f32,
+    pub x: T,
     /// Vertical change
     #[serde(default, deserialize_with = "failure_default")]
-    pub y: f32,
-}
-
-impl Default for Delta {
-    fn default() -> Delta {
-        Delta { x: 0.0, y: 0.0 }
-    }
+    pub y: T,
 }
 
 trait DeserializeSize : Sized {
@@ -1539,11 +1534,11 @@ pub struct Font {
 
     /// Extra spacing per character
     #[serde(default, deserialize_with = "failure_default")]
-    offset: Delta,
+    offset: Delta<i8>,
 
     /// Glyph offset within character cell
     #[serde(default, deserialize_with = "failure_default")]
-    glyph_offset: Delta,
+    glyph_offset: Delta<i8>,
 
     #[serde(default="true_bool", deserialize_with = "default_true_bool")]
     use_thin_strokes: bool
@@ -1582,13 +1577,13 @@ impl Font {
 
     /// Get offsets to font metrics
     #[inline]
-    pub fn offset(&self) -> &Delta {
+    pub fn offset(&self) -> &Delta<i8> {
         &self.offset
     }
 
     /// Get cell offsets for glyphs
     #[inline]
-    pub fn glyph_offset(&self) -> &Delta {
+    pub fn glyph_offset(&self) -> &Delta<i8> {
         &self.glyph_offset
     }
 
diff --git a/src/display.rs b/src/display.rs
index 418f616e..bd8ae401 100644
--- a/src/display.rs
+++ b/src/display.rs
@@ -178,8 +178,8 @@ impl Display {
             height: viewport_size.height.0 as f32,
             cell_width: cell_width as f32,
             cell_height: cell_height as f32,
-            padding_x: config.padding().x.floor(),
-            padding_y: config.padding().y.floor(),
+            padding_x: config.padding().x as f32,
+            padding_y: config.padding().y as f32,
         };
 
         // Channel for resize events
@@ -238,10 +238,15 @@ impl Display {
         // font metrics should be computed before creating the window in the first
         // place so that a resize is not needed.
         let metrics = glyph_cache.font_metrics();
-        let cell_width = (metrics.average_advance + f64::from(font.offset().x)) as u32;
-        let cell_height = (metrics.line_height + f64::from(font.offset().y)) as u32;
+        let cell_width = metrics.average_advance as f32 + font.offset().x as f32;
+        let cell_height = metrics.line_height as f32 + font.offset().y as f32;
 
-        Ok((glyph_cache, cell_width as f32, cell_height as f32))
+        // Prevent invalid cell sizes
+        if cell_width < 1. || cell_height < 1. {
+            panic!("font offset is too small");
+        }
+
+        Ok((glyph_cache, cell_width.floor(), cell_height.floor()))
     }
 
     pub fn update_glyph_cache(&mut self, config: &Config) {
diff --git a/src/renderer/mod.rs b/src/renderer/mod.rs
index 5b53e70f..56ae4b14 100644
--- a/src/renderer/mod.rs
+++ b/src/renderer/mod.rs
@@ -123,8 +123,8 @@ pub struct ShaderProgram {
     /// Rendering is split into two passes; 1 for backgrounds, and one for text
     u_background: GLint,
 
-    padding_x: f32,
-    padding_y: f32,
+    padding_x: u8,
+    padding_y: u8,
 }
 
 
@@ -165,7 +165,7 @@ pub struct GlyphCache {
     font_size: font::Size,
 
     /// glyph offset
-    glyph_offset: Delta,
+    glyph_offset: Delta<i8>,
 
     metrics: ::font::Metrics,
 }
@@ -1017,8 +1017,8 @@ impl ShaderProgram {
             u_cell_dim: cell_dim,
             u_visual_bell: visual_bell,
             u_background: background,
-            padding_x: config.padding().x.floor(),
-            padding_y: config.padding().y.floor(),
+            padding_x: config.padding().x,
+            padding_y: config.padding().y,
         };
 
         shader.update_projection(*size.width as f32, *size.height as f32);
@@ -1041,8 +1041,8 @@ impl ShaderProgram {
         // NB Not sure why padding change only requires changing the vertical
         //    translation in the projection, but this makes everything work
         //    correctly.
-        let ortho = cgmath::ortho(0., width - 2. * self.padding_x, 2. * self.padding_y, height,
-            -1., 1.);
+        let ortho = cgmath::ortho(0., width - 2. * self.padding_x as f32, 2. * self.padding_y as f32,
+            height, -1., 1.);
         let projection: [[f32; 4]; 4] = ortho.into();
 
         info!("width: {}, height: {}", width, height);