From 5602cfa2ee87ae40b2b6bf56b3a24eba7c9b51cb Mon Sep 17 00:00:00 2001 From: Christopher Pitstick Date: Sun, 20 Mar 2022 02:55:09 -0400 Subject: [PATCH] Updating to refactor xrdp_client_info - Eliminate duplicaiton for display_size_description - monitorCount needs to be uint32_t - width/height -> session_width/session_height - Update CLIENT_INFO_CURRENT_VERSION --- common/xrdp_client_info.h | 10 ++--- libxrdp/libxrdp.c | 15 +++---- libxrdp/libxrdp.h | 6 ++- libxrdp/xrdp_caps.c | 29 +++++++------- libxrdp/xrdp_sec.c | 18 ++++----- tests/libxrdp/test_monitor_processing.c | 52 ++++++++++++------------- vnc/vnc.c | 28 ++++++------- xrdp/xrdp_login_wnd.c | 16 ++++---- xrdp/xrdp_mm.c | 20 +++++----- xrdp/xrdp_wm.c | 16 ++++---- 10 files changed, 106 insertions(+), 104 deletions(-) diff --git a/common/xrdp_client_info.h b/common/xrdp_client_info.h index 106e47a7e3..07e2e10557 100644 --- a/common/xrdp_client_info.h +++ b/common/xrdp_client_info.h @@ -59,7 +59,7 @@ struct xrdp_keyboard_overrides struct display_size_description { - int monitorCount; /* number of monitors detected (max = 16) */ + unsigned int monitorCount; /* 2.2.2.2 DISPLAYCONTROL_MONITOR_LAYOUT_PDU: number of monitors detected (max = 16) */ struct monitor_info minfo[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data */ struct monitor_info minfo_wm[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data, non-negative values */ int session_width; @@ -79,8 +79,6 @@ struct xrdp_client_info int size; /* bytes for this structure */ int version; /* Should be CLIENT_INFO_CURRENT_VERSION */ int bpp; - int width; - int height; /* bitmap cache info */ int cache1_entries; int cache1_size; @@ -153,9 +151,7 @@ struct xrdp_client_info int security_layer; /* 0 = rdp, 1 = tls , 2 = hybrid */ int multimon; /* 0 = deny , 1 = allow */ - int monitorCount; /* number of monitors detected (max = 16) */ - struct monitor_info minfo[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data */ - struct monitor_info minfo_wm[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data, non-negative values */ + struct display_size_description display_sizes; int keyboard_type; int keyboard_subtype; @@ -211,6 +207,6 @@ struct xrdp_client_info }; /* yyyymmdd of last incompatible change to xrdp_client_info */ -#define CLIENT_INFO_CURRENT_VERSION 20210723 +#define CLIENT_INFO_CURRENT_VERSION 20220320 #endif diff --git a/libxrdp/libxrdp.c b/libxrdp/libxrdp.c index f25b18c288..82e54a88c0 100644 --- a/libxrdp/libxrdp.c +++ b/libxrdp/libxrdp.c @@ -1147,18 +1147,18 @@ libxrdp_reset(struct xrdp_session *session, } /* if same (and only one monitor on client) don't need to do anything */ - if (client_info->width == width && - client_info->height == height && + if (client_info->display_sizes.session_width == width && + client_info->display_sizes.session_height == height && client_info->bpp == bpp && - (client_info->monitorCount == 0 || client_info->multimon == 0)) + (client_info->display_sizes.monitorCount == 0 || client_info->multimon == 0)) { return 0; } - client_info->width = width; - client_info->height = height; + client_info->display_sizes.session_width = width; + client_info->display_sizes.session_height = height; + client_info->display_sizes.monitorCount = 0; client_info->bpp = bpp; - client_info->monitorCount = 0; client_info->multimon = 0; } else @@ -1909,7 +1909,8 @@ libxrdp_process_monitor_stream(struct stream *s, in_uint32_le(s, monitor_layout->physical_height); in_uint32_le(s, monitor_layout->orientation); - switch (monitor_layout->orientation) { + switch (monitor_layout->orientation) + { case ORIENTATION_LANDSCAPE: case ORIENTATION_PORTRAIT: case ORIENTATION_LANDSCAPE_FLIPPED: diff --git a/libxrdp/libxrdp.h b/libxrdp/libxrdp.h index fb931bf3d0..b124a85bcc 100644 --- a/libxrdp/libxrdp.h +++ b/libxrdp/libxrdp.h @@ -361,8 +361,10 @@ xrdp_mcs_disconnect(struct xrdp_mcs *self); /* xrdp_sec.c */ /* - These are return values for xrdp_sec_process_mcs_data_monitors - to clarify any reason for a non-zero response code. + These are error return codes for both: + 1. xrdp_sec_process_mcs_data_monitors + 2. libxrdp_process_monitor_stream + To clarify any reason for a non-zero response code. */ #define SEC_PROCESS_MONITORS_ERR 1 #define SEC_PROCESS_MONITORS_ERR_TOO_MANY_MONITORS 2 diff --git a/libxrdp/xrdp_caps.c b/libxrdp/xrdp_caps.c index 5c5e74a579..c953205342 100644 --- a/libxrdp/xrdp_caps.c +++ b/libxrdp/xrdp_caps.c @@ -45,7 +45,8 @@ static int xrdp_caps_send_monitorlayout(struct xrdp_rdp *self) { struct stream *s; - int i; + uint32_t i; + struct display_size_description *description; make_stream(s); init_stream(s, 8192); @@ -56,16 +57,18 @@ xrdp_caps_send_monitorlayout(struct xrdp_rdp *self) return 1; } - out_uint32_le(s, self->client_info.monitorCount); /* monitorCount (4 bytes) */ + description = &self->client_info.display_sizes; + + out_uint32_le(s, description->monitorCount); /* monitorCount (4 bytes) */ /* TODO: validate for allowed monitors in terminal server (maybe by config?) */ - for (i = 0; i < self->client_info.monitorCount; i++) + for (i = 0; i < description->monitorCount; i++) { - out_uint32_le(s, self->client_info.minfo[i].left); - out_uint32_le(s, self->client_info.minfo[i].top); - out_uint32_le(s, self->client_info.minfo[i].right); - out_uint32_le(s, self->client_info.minfo[i].bottom); - out_uint32_le(s, self->client_info.minfo[i].is_primary); + out_uint32_le(s, description->minfo[i].left); + out_uint32_le(s, description->minfo[i].top); + out_uint32_le(s, description->minfo[i].right); + out_uint32_le(s, description->minfo[i].bottom); + out_uint32_le(s, description->minfo[i].is_primary); } s_mark_end(s); @@ -883,8 +886,8 @@ unsigned int calculate_multifragmentupdate_len(const struct xrdp_rdp *self) { unsigned int result = MAX_MULTIFRAGMENTUPDATE_SIZE; - unsigned int x_tiles = (self->client_info.width + 63) / 64; - unsigned int y_tiles = (self->client_info.height + 63) / 64; + unsigned int x_tiles = (self->client_info.display_sizes.session_width + 63) / 64; + unsigned int y_tiles = (self->client_info.display_sizes.session_height + 63) / 64; /* Check for overflow on calculation if bad parameters are supplied */ if ((x_tiles * y_tiles + 1) < (UINT_MAX / 16384)) @@ -979,8 +982,8 @@ xrdp_caps_send_demand_active(struct xrdp_rdp *self) out_uint16_le(s, 1); /* Receive 1 BPP */ out_uint16_le(s, 1); /* Receive 4 BPP */ out_uint16_le(s, 1); /* Receive 8 BPP */ - out_uint16_le(s, self->client_info.width); /* width */ - out_uint16_le(s, self->client_info.height); /* height */ + out_uint16_le(s, self->client_info.display_sizes.session_width); /* width */ + out_uint16_le(s, self->client_info.display_sizes.session_height); /* height */ out_uint16_le(s, 0); /* Pad */ out_uint16_le(s, 1); /* Allow resize */ out_uint16_le(s, 1); /* bitmap compression */ @@ -1242,7 +1245,7 @@ xrdp_caps_send_demand_active(struct xrdp_rdp *self) } /* send Monitor Layout PDU for dual monitor */ - if (self->client_info.monitorCount > 0 && + if (self->client_info.display_sizes.monitorCount > 0 && self->client_info.multimon == 1) { LOG_DEVEL(LOG_LEVEL_TRACE, "xrdp_caps_send_demand_active: sending monitor layout pdu"); diff --git a/libxrdp/xrdp_sec.c b/libxrdp/xrdp_sec.c index ba3ad70c61..885b5a3d47 100644 --- a/libxrdp/xrdp_sec.c +++ b/libxrdp/xrdp_sec.c @@ -1953,8 +1953,8 @@ xrdp_sec_process_mcs_data_CS_CORE(struct xrdp_sec *self, struct stream *s) /* TS_UD_CS_CORE requiered fields */ in_uint8s(s, 4); /* version */ - in_uint16_le(s, self->rdp_layer->client_info.width); - in_uint16_le(s, self->rdp_layer->client_info.height); + in_uint16_le(s, self->rdp_layer->client_info.display_sizes.session_width); + in_uint16_le(s, self->rdp_layer->client_info.display_sizes.session_height); in_uint16_le(s, colorDepth); switch (colorDepth) { @@ -1981,8 +1981,8 @@ xrdp_sec_process_mcs_data_CS_CORE(struct xrdp_sec *self, struct stream *s) "clientName %s, keyboardType (ignored), " "keyboardSubType (ignored), keyboardFunctionKey (ignored), " "imeFileName (ignroed)", - self->rdp_layer->client_info.width, - self->rdp_layer->client_info.height, + self->rdp_layer->client_info.display_sizes.session_width, + self->rdp_layer->client_info.display_sizes.session_height, (colorDepth == 0xca00 ? "RNS_UD_COLOR_4BPP" : colorDepth == 0xca01 ? "RNS_UD_COLOR_8BPP" : "unknown"), clientName); @@ -2370,17 +2370,17 @@ xrdp_sec_process_mcs_data_monitors(struct xrdp_sec *self, struct stream *s) error = libxrdp_process_monitor_stream(s, description, 0); if (error == 0) { - client_info->monitorCount = description->monitorCount; + client_info->display_sizes.monitorCount = description->monitorCount; LOG_DEVEL(LOG_LEVEL_TRACE, "xrdp_sec_process_mcs_data_monitors:" " Received [MS-RDPBCGR] TS_UD_CS_MONITOR" " flags 0x%8.8x, monitorCount %d", flags, description->monitorCount); - client_info->width = description->session_width; - client_info->height = description->session_height; - g_memcpy(client_info->minfo, description->minfo, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS); - g_memcpy(client_info->minfo_wm, description->minfo_wm, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS); + client_info->display_sizes.session_width = description->session_width; + client_info->display_sizes.session_height = description->session_height; + g_memcpy(client_info->display_sizes.minfo, description->minfo, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS); + g_memcpy(client_info->display_sizes.minfo_wm, description->minfo_wm, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS); } g_free(description); diff --git a/tests/libxrdp/test_monitor_processing.c b/tests/libxrdp/test_monitor_processing.c index 0314e7ddf6..331e2f858a 100644 --- a/tests/libxrdp/test_monitor_processing.c +++ b/tests/libxrdp/test_monitor_processing.c @@ -92,25 +92,25 @@ START_TEST(test_process_monitors__with_single_monitor_happy_path) int error = xrdp_sec_process_mcs_data_monitors(sec_layer, s); ck_assert_int_eq(error, 0); - ck_assert_int_eq(client_info->monitorCount, 1); + ck_assert_int_eq(client_info->display_sizes.monitorCount, 1); // Verify normal monitor - ck_assert_int_eq(client_info->minfo[0].left, 0); - ck_assert_int_eq(client_info->minfo[0].top, 0); - ck_assert_int_eq(client_info->minfo[0].right, 3840); - ck_assert_int_eq(client_info->minfo[0].bottom, 2160); - ck_assert_int_eq(client_info->minfo[0].is_primary, 1); + ck_assert_int_eq(client_info->display_sizes.minfo[0].left, 0); + ck_assert_int_eq(client_info->display_sizes.minfo[0].top, 0); + ck_assert_int_eq(client_info->display_sizes.minfo[0].right, 3840); + ck_assert_int_eq(client_info->display_sizes.minfo[0].bottom, 2160); + ck_assert_int_eq(client_info->display_sizes.minfo[0].is_primary, 1); // Verify normalized monitor - ck_assert_int_eq(client_info->minfo_wm[0].left, 0); - ck_assert_int_eq(client_info->minfo_wm[0].top, 0); - ck_assert_int_eq(client_info->minfo_wm[0].right, 3840); - ck_assert_int_eq(client_info->minfo_wm[0].bottom, 2160); - ck_assert_int_eq(client_info->minfo_wm[0].is_primary, 1); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].left, 0); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].top, 0); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].right, 3840); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].bottom, 2160); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].is_primary, 1); // Verify geometry (+1 greater than ) - ck_assert_int_eq(client_info->width, 3841); - ck_assert_int_eq(client_info->height, 2161); + ck_assert_int_eq(client_info->display_sizes.session_width, 3841); + ck_assert_int_eq(client_info->display_sizes.session_height, 2161); free_stream(s); } @@ -141,25 +141,25 @@ START_TEST(test_process_monitors__when_no_primary_monitor_is_specified_one_is_se int error = xrdp_sec_process_mcs_data_monitors(sec_layer, s); ck_assert_int_eq(error, 0); - ck_assert_int_eq(client_info->monitorCount, 1); + ck_assert_int_eq(client_info->display_sizes.monitorCount, 1); // Verify normal monitor - ck_assert_int_eq(client_info->minfo[0].left, 0); - ck_assert_int_eq(client_info->minfo[0].top, 0); - ck_assert_int_eq(client_info->minfo[0].right, 3840); - ck_assert_int_eq(client_info->minfo[0].bottom, 2160); - ck_assert_int_eq(client_info->minfo[0].is_primary, 1); + ck_assert_int_eq(client_info->display_sizes.minfo[0].left, 0); + ck_assert_int_eq(client_info->display_sizes.minfo[0].top, 0); + ck_assert_int_eq(client_info->display_sizes.minfo[0].right, 3840); + ck_assert_int_eq(client_info->display_sizes.minfo[0].bottom, 2160); + ck_assert_int_eq(client_info->display_sizes.minfo[0].is_primary, 1); // Verify normalized monitor - ck_assert_int_eq(client_info->minfo_wm[0].left, 0); - ck_assert_int_eq(client_info->minfo_wm[0].top, 0); - ck_assert_int_eq(client_info->minfo_wm[0].right, 3840); - ck_assert_int_eq(client_info->minfo_wm[0].bottom, 2160); - ck_assert_int_eq(client_info->minfo_wm[0].is_primary, 1); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].left, 0); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].top, 0); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].right, 3840); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].bottom, 2160); + ck_assert_int_eq(client_info->display_sizes.minfo_wm[0].is_primary, 1); // Verify geometry (+1 greater than ) - ck_assert_int_eq(client_info->width, 3841); - ck_assert_int_eq(client_info->height, 2161); + ck_assert_int_eq(client_info->display_sizes.session_width, 3841); + ck_assert_int_eq(client_info->display_sizes.session_height, 2161); free_stream(s); } diff --git a/vnc/vnc.c b/vnc/vnc.c index 7486132981..cf2c6e796d 100644 --- a/vnc/vnc.c +++ b/vnc/vnc.c @@ -2039,25 +2039,25 @@ static void init_client_layout(struct vnc_screen_layout *layout, const struct xrdp_client_info *client_info) { - int i; + uint32_t i; - layout->total_width = client_info->width; - layout->total_height = client_info->height; + layout->total_width = client_info->display_sizes.session_width; + layout->total_height = client_info->display_sizes.session_height; - layout->count = client_info->monitorCount; + layout->count = client_info->display_sizes.monitorCount; layout->s = g_new(struct vnc_screen, layout->count); - for (i = 0 ; i < client_info->monitorCount ; ++i) + for (i = 0 ; i < client_info->display_sizes.monitorCount ; ++i) { /* Use minfo_wm, as this is normalised for a top-left of (0,0) * as required by RFC6143 */ layout->s[i].id = i; - layout->s[i].x = client_info->minfo_wm[i].left; - layout->s[i].y = client_info->minfo_wm[i].top; - layout->s[i].width = client_info->minfo_wm[i].right - - client_info->minfo_wm[i].left + 1; - layout->s[i].height = client_info->minfo_wm[i].bottom - - client_info->minfo_wm[i].top + 1; + layout->s[i].x = client_info->display_sizes.minfo_wm[i].left; + layout->s[i].y = client_info->display_sizes.minfo_wm[i].top; + layout->s[i].width = client_info->display_sizes.minfo_wm[i].right - + client_info->display_sizes.minfo_wm[i].left + 1; + layout->s[i].height = client_info->display_sizes.minfo_wm[i].bottom - + client_info->display_sizes.minfo_wm[i].top + 1; layout->s[i].flags = 0; } } @@ -2106,11 +2106,11 @@ lib_mod_set_param(struct vnc *v, const char *name, const char *value) g_free(v->client_layout.s); /* Save monitor information from the client */ - if (!client_info->multimon || client_info->monitorCount < 1) + if (!client_info->multimon || client_info->display_sizes.monitorCount < 1) { set_single_screen_layout(&v->client_layout, - client_info->width, - client_info->height); + client_info->display_sizes.session_width, + client_info->display_sizes.session_height); } else { diff --git a/xrdp/xrdp_login_wnd.c b/xrdp/xrdp_login_wnd.c index da5917b232..28ab4f851d 100644 --- a/xrdp/xrdp_login_wnd.c +++ b/xrdp/xrdp_login_wnd.c @@ -652,7 +652,7 @@ xrdp_login_wnd_create(struct xrdp_wm *self) int primary_height; int primary_x_offset; /* Offset of centre of primary screen */ int primary_y_offset; - int index; + uint32_t index; int x; int y; int cx; @@ -684,16 +684,16 @@ xrdp_login_wnd_create(struct xrdp_wm *self) } /* multimon scenario, draw login window on primary monitor */ - if (self->client_info->monitorCount > 1) + if (self->client_info->display_sizes.monitorCount > 1) { - for (index = 0; index < self->client_info->monitorCount; index++) + for (index = 0; index < self->client_info->display_sizes.monitorCount; index++) { - if (self->client_info->minfo_wm[index].is_primary) + if (self->client_info->display_sizes.minfo_wm[index].is_primary) { - x = self->client_info->minfo_wm[index].left; - y = self->client_info->minfo_wm[index].top; - cx = self->client_info->minfo_wm[index].right; - cy = self->client_info->minfo_wm[index].bottom; + x = self->client_info->display_sizes.minfo_wm[index].left; + y = self->client_info->display_sizes.minfo_wm[index].top; + cx = self->client_info->display_sizes.minfo_wm[index].right; + cy = self->client_info->display_sizes.minfo_wm[index].bottom; primary_width = cx - x; primary_height = cy - y; diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index c10f1b5d8c..145369e71d 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1144,14 +1144,14 @@ process_dynamic_monitor_description(struct xrdp_wm *wm, } } - wm->client_info->monitorCount = description->monitorCount; - wm->client_info->width = description->session_width; - wm->client_info->height = description->session_height; - g_memcpy(wm->client_info->minfo, + wm->client_info->display_sizes.monitorCount = description->monitorCount; + wm->client_info->display_sizes.session_width = description->session_width; + wm->client_info->display_sizes.session_height = description->session_height; + g_memcpy(wm->client_info->display_sizes.minfo, description->minfo, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS); - g_memcpy(wm->client_info->minfo_wm, + g_memcpy(wm->client_info->display_sizes.minfo_wm, description->minfo_wm, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS); @@ -3603,10 +3603,10 @@ server_reset(struct xrdp_mod *mod, int width, int height, int bpp) } /* if same (and only one monitor on client) don't need to do anything */ - if (wm->client_info->width == width && - wm->client_info->height == height && + if (wm->client_info->display_sizes.session_width == width && + wm->client_info->display_sizes.session_height == height && wm->client_info->bpp == bpp && - (wm->client_info->monitorCount == 0 || wm->client_info->multimon == 0)) + (wm->client_info->display_sizes.monitorCount == 0 || wm->client_info->multimon == 0)) { return 0; } @@ -3620,8 +3620,8 @@ server_reset(struct xrdp_mod *mod, int width, int height, int bpp) /* reset cache */ xrdp_cache_reset(wm->cache, wm->client_info); /* resize the main window */ - xrdp_bitmap_resize(wm->screen, wm->client_info->width, - wm->client_info->height); + xrdp_bitmap_resize(wm->screen, wm->client_info->display_sizes.session_width, + wm->client_info->display_sizes.session_height); /* load some stuff */ xrdp_wm_load_static_colors_plus(wm, 0); xrdp_wm_load_static_pointers(wm); diff --git a/xrdp/xrdp_wm.c b/xrdp/xrdp_wm.c index 94691582d0..caa1160a0c 100644 --- a/xrdp/xrdp_wm.c +++ b/xrdp/xrdp_wm.c @@ -45,8 +45,8 @@ xrdp_wm_create(struct xrdp_process *owner, self = (struct xrdp_wm *)g_malloc(sizeof(struct xrdp_wm), 1); self->client_info = client_info; - self->screen = xrdp_bitmap_create(client_info->width, - client_info->height, + self->screen = xrdp_bitmap_create(client_info->display_sizes.session_width, + client_info->display_sizes.session_height, client_info->bpp, WND_TYPE_SCREEN, self); self->screen->wm = self; @@ -2088,7 +2088,7 @@ xrdp_wm_show_log(struct xrdp_wm *self) int h; int xoffset; int yoffset; - int index; + uint32_t index; int primary_x_offset; int primary_y_offset; @@ -2124,14 +2124,14 @@ xrdp_wm_show_log(struct xrdp_wm *self) primary_y_offset = 0; /* multimon scenario, draw log window on primary monitor */ - if (self->client_info->monitorCount > 1) + if (self->client_info->display_sizes.monitorCount > 1) { - for (index = 0; index < self->client_info->monitorCount; index++) + for (index = 0; index < self->client_info->display_sizes.monitorCount; index++) { - if (self->client_info->minfo_wm[index].is_primary) + if (self->client_info->display_sizes.minfo_wm[index].is_primary) { - primary_x_offset = self->client_info->minfo_wm[index].left; - primary_y_offset = self->client_info->minfo_wm[index].top; + primary_x_offset = self->client_info->display_sizes.minfo_wm[index].left; + primary_y_offset = self->client_info->display_sizes.minfo_wm[index].top; break; } }