(#2509) Handling integer overflows for dimensions for XWaylandSurface

[mattkae]

#2509

You can find some more info on the solution to this one in the ticket itself ☝️

How to test

• Run miral-app or something
• Open up mattermost
• Click the fullscreen button or resize it using the edges
• Note that this now works!

Solution

• I am going to assume that the width and height are never naturally negative. Hence, if we overflow during the multiplication, we can expect the int to go negative.
• This might just be an unsafe multiplication all-around, especially if we have a negative scale (e.g. a scale of 0.25 would result in an inv_scale of 8)

Screen recording

Screencast.from.09-21-2023.05.02.28.PM.webm

[codecov]

Codecov Report

Merging #3049 (eac793b) into main (da0c8fa) will increase coverage by 0.00%.
Report is 2 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3049   +/-   ##
=======================================
  Coverage   77.70%   77.70%           
=======================================
  Files        1056     1056           
  Lines       73403    73403           
=======================================
+ Hits        57039    57041    +2     
+ Misses      16364    16362    -2     

Files Changed	Coverage Δ
src/server/frontend_xwayland/xwayland_surface.cpp	0.00% <ø> (ø)

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[RAOF]

Hm. I think this might be better handled in the converting Value constructors? (geometry/dimensions.h lines 72 and 78)

Rather than just YOLOing static_cast<> there, do a saturating cast? (ie: check value against std::numeric_limits<T> and return ::max/::min as appropriate).

There's never a case where we want UB there (which is the current situation) and I don't think there's ever a case where we'd want anything other than saturation?

[AlanGriffiths]

Thanks for tracking this down, but I don't like this solution either. And I'm not sure the converting constructor is the place either. See also #3050