Blob Blame History Raw
From 9a861cb3e5e7bd58dc0d4aa549c6d3df25f8b24d Mon Sep 17 00:00:00 2001
From: Marius Kittler <mkittler@suse.de>
Date: Fri, 17 Jun 2022 17:29:20 +0200
Subject: [PATCH] Fix using little-endian VNC server on big-endian

* Fix connecting to a little-endian VNC server from a big-endian
  system
* Use byte swap functions instead of custom code to swap the
  byte order
    * The bswap_32()/bswap_16() functions have the same
      behavior as our custom code on little-endian systems and
      therefore this change should not do anything on
      little-endian systems.
    * The bswap_32()/bswap_16() functions change the byte order
      also on big-endian systems (in contrast to our custom
      code).
* The VNC consoles test has been updated to test both cases
  (byte order swap necassary and not necassary). Before it
  only tested the case of a little-endian VNC server (and was
  therefore only failing on bit-endian systems before this
  change).
* Tested the behavior on x86_64 and also on a Tumbleweed
  ppc64 VM.
* See https://progress.opensuse.org/issues/111608.
---
 ppmclibs/tinycv_impl.cc | 23 +++++++----------------
 t/27-consoles-vnc.t     | 14 +++++++++++++-
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/ppmclibs/tinycv_impl.cc b/ppmclibs/tinycv_impl.cc
index c987127d5d..93f40571e8 100644
--- a/ppmclibs/tinycv_impl.cc
+++ b/ppmclibs/tinycv_impl.cc
@@ -10,6 +10,7 @@
 #include <cstdint>
 #include <cstdio>
 #include <sys/time.h>
+#include <byteswap.h>
 
 #include <algorithm> // std::min
 #include <vector>
@@ -647,13 +648,10 @@ void image_map_raw_data_rgb555(Image* a, const unsigned char* data)
 static uint16_t read_u16(const unsigned char* data, size_t& offset,
     bool do_endian_conversion)
 {
-    uint16_t pixel;
+    uint16_t pixel = *(uint16_t*)(data + offset);
+    offset += 2;
     if (do_endian_conversion) {
-        pixel = data[offset++] * 256;
-        pixel += data[offset++];
-    } else {
-        pixel = data[offset++];
-        pixel += data[offset++] * 256;
+        pixel = bswap_16(pixel);
     }
     return pixel;
 }
@@ -668,17 +666,10 @@ Vec3b VNCInfo::read_pixel(const unsigned char* data, size_t& offset)
     if (bytes_per_pixel == 2) {
         pixel = read_u16(data, offset, do_endian_conversion);
     } else if (bytes_per_pixel == 4) {
+        pixel = *(uint32_t*)(data + offset);
+        offset += 4;
         if (do_endian_conversion) {
-            pixel = data[offset++];
-            pixel <<= 8;
-            pixel |= data[offset++];
-            pixel <<= 8;
-            pixel |= data[offset++];
-            pixel <<= 8;
-            pixel |= data[offset++];
-        } else {
-            pixel = *(uint32_t*)(data + offset);
-            offset += 4;
+            pixel = bswap_32(pixel);
         }
     } else if (bytes_per_pixel == 1) {
         pixel = data[offset++];
diff --git a/t/27-consoles-vnc.t b/t/27-consoles-vnc.t
index 455afc0646..be0731c9d6 100755
--- a/t/27-consoles-vnc.t
+++ b/t/27-consoles-vnc.t
@@ -41,7 +41,6 @@ is_deeply \@printed, ['RFB 003.006', pack('C', 1)], 'protocol version and securi
 
 # ensure endian conversion is setup correctly (despite initially mocking _server_initialization)
 my $machine_is_big_endian = unpack('h*', pack('s', 1)) =~ /01/ ? 1 : 0;
-$c->_do_endian_conversion($machine_is_big_endian);
 
 subtest 'send update request' => sub {
     $c->width(1024);
@@ -161,6 +160,7 @@ subtest 'update framebuffer' => sub {
     ok $logged_in, 'relogin on protocol error';
 
     # test with full data (just one pixel, though) and vncinfo present (defines endianness and chroma subsampling)
+    $c->_do_endian_conversion($machine_is_big_endian);    # assume server is little-endian
     my $vncinfo = tinycv::new_vncinfo($c->_do_endian_conversion, $c->_true_colour, $c->_bpp / 8, 255, 0, 255, 8, 255, 16);
     my $gray_pixel = pack(CCCC => 31, 37, 41, 0);    # dark prime grey
     my $of_type_raw_with_coordinates_43_47_1_1 = pack(nnnnN => 43, 47, 1, 1, 0);
@@ -181,6 +181,18 @@ subtest 'update framebuffer' => sub {
     throws_ok { $c->update_framebuffer } qr/unsupported update encoding -225/, 'dies on unsupported encoding';
     is $s->mocked_read, undef, 'no more messages left to read after reading unknown encoding';
 
+    # test with full data again, assuming the server is big-endian
+    $c->_do_endian_conversion(!$machine_is_big_endian);    # assume server is big-endian
+    $vncinfo = tinycv::new_vncinfo($c->_do_endian_conversion, $c->_true_colour, $c->_bpp / 8, 255, 0, 255, 8, 255, 16);
+    $gray_pixel = pack(CCCC => 0, 41, 37, 31);    # dark prime grey
+    $s->set_series(mocked_read => $update_message, $one_rectangle, $of_type_raw_with_coordinates_43_47_1_1, $gray_pixel);
+    $c->_framebuffer(undef)->width(1024)->height(512)->vncinfo($vncinfo);
+    ok $c->update_framebuffer, 'truthy return value for successful pixel update of big-endian server';
+    ($blue, $green, $red) = $c->_framebuffer->get_pixel(43, 47);
+    is $blue, 41, 'pixel data updated in framebuffer (blue, big-endian server)';
+    is $green, 37, 'pixel data updated in framebuffer (green, big-endian server)';
+    is $red, 31, 'pixel data updated in framebuffer (red, big-endian server)';
+
     $c->ikvm(1);
     my $unsupported_ikvm_encoding = pack(nnnnN => 0, 0, 1, 1, 88);
     my $ikvm_specific_data = pack(NN => 0, 9);    # some "prefix" and data length