Skip to content

Parse more physical monitor size information #2310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matt335672 merged 1 commit into from
Aug 1, 2022
Merged

Parse more physical monitor size information #2310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions common/ms-rdpbcgr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#define SEC_TAG_CLI_CHANNELS 0xc003 /* CS_CHANNELS? */
#define SEC_TAG_CLI_4 0xc004 /* CS_CLUSTER? */
#define SEC_TAG_CLI_MONITOR 0xc005 /* CS_MONITOR */
#define SEC_TAG_CLI_MONITOR_EX 0xc008 /* CS_MONITOR_EX */

/* Client Core Data: colorDepth, postBeta2ColorDepth (2.2.1.3.2) */
#define RNS_UD_COLOR_4BPP 0xCA00
Expand Down
10 changes: 9 additions & 1 deletion common/xrdp_client_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ struct monitor_info
int bottom;
int flags;

/* From 2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT */
/* From [MS-RDPEDISP] 2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT, or
* [MS-RDPBCGR] 2.2.1.3.9.1 (TS_MONITOR_ATTRIBUTES) */
unsigned int physical_width;
unsigned int physical_height;
unsigned int orientation;
Expand Down Expand Up @@ -205,6 +206,13 @@ struct xrdp_client_info

/* xrdp.override_* values */
struct xrdp_keyboard_overrides xrdp_keyboard_overrides;

/* These values are optionally send over as part of TS_UD_CS_CORE.
* They can be used as a fallback for a single monitor session
* if physical sizes are not available in the monitor-specific
* data */
unsigned int session_physical_width; /* in mm */
Copy link
Contributor

@Nexarian Nexarian Jul 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not put session_physical_width (and height) into display_size_description? It contains the information for all the monitors in addition to summary information such as the size of the full session.

Also, this doesn't seem to have logic that would be at parity with session_height/width -- Those are for computing the height/width of the ENTIRE session. Is there a similar computation for the physical width/height of the ENTIRE session when it's provided for all monitors?

I think also, how do we deal with DPI if multiple monitors are provided that each have a different scale factor? Pick the highest DPI? Lowest?

Copy link
Member Author

@matt335672 matt335672 Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funnily enough I started down that route. Eventually however it occurred to me that it didn't make much sense.

The physical size of the desktop is easy to understand for a single monitor, and actually has a use, which is to calculate the DPI for the monitor (in the absence of any other information). For multiple monitors however, which may not even be touching in any way, the overall physical size is not something that can be defined. For example, what would you do in a situation like this with a gap between the monitors?

+---------+       +---------+
|         |       |         |
|    0    |       |    1    |
|         |       |         |
+---------+       +---------+

So eventually I removed the overall physical size from the display_size_description, and just left the values from the CORE message further down the structure as a fallback, in case extended monitor information isn't available. These could be renamed maybe, if it's not clear what is going on.

Dealing with the DPI on the login screen is easy - the login screen only appears on the primary monitor, so that's the only monitor we need to worry about.

unsigned int session_physical_height; /* in mm */
};

/* yyyymmdd of last incompatible change to xrdp_client_info */
Expand Down
273 changes: 196 additions & 77 deletions libxrdp/libxrdp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "ms-rdpbcgr.h"

#define MAX_BITMAP_BUF_SIZE (16 * 1024) /* 16K */
#define TS_MONITOR_ATTRIBUTES_SIZE 20 /* [MS-RDPBCGR] 2.2.1.3.9 */

/******************************************************************************/
struct xrdp_session *EXPORT_CC
Expand Down Expand Up @@ -1764,6 +1765,97 @@ libxrdp_send_session_info(struct xrdp_session *session, const char *data,
return xrdp_rdp_send_session_info(rdp, data, data_bytes);
}

/*****************************************************************************/
/*
Sanitise extended monitor attributes

The extended attributes are received from either
[MS-RDPEDISP] 2.2.2.2.1 (DISPLAYCONTROL_MONITOR_LAYOUT), or
[MS-RDPBCGR] 2.2.1.3.9.1 (TS_MONITOR_ATTRIBUTES)

@param monitor_layout struct containing extended attributes
*/
static void
sanitise_extended_monitor_attributes(struct monitor_info *monitor_layout)
Copy link
Contributor

@Nexarian Nexarian Jul 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this refactor, especially because it helps clean up and clarify what extended monitor attributes are!

{
/* if EITHER physical_width or physical_height are
* out of range, BOTH must be ignored.
*/
if (monitor_layout->physical_width > 10000
|| monitor_layout->physical_width < 10)
{
LOG(LOG_LEVEL_WARNING, "sanitise_extended_monitor_attributes:"
" physical_width is not within valid range."
" Setting physical_width to 0mm,"
" Setting physical_height to 0mm,"
" physical_width was: %d",
monitor_layout->physical_width);
monitor_layout->physical_width = 0;
monitor_layout->physical_height = 0;
}

if (monitor_layout->physical_height > 10000
|| monitor_layout->physical_height < 10)
{
LOG(LOG_LEVEL_WARNING, "sanitise_extended_monitor_attributes:"
" physical_height is not within valid range."
" Setting physical_width to 0mm,"
" Setting physical_height to 0mm,"
" physical_height was: %d",
monitor_layout->physical_height);
monitor_layout->physical_width = 0;
monitor_layout->physical_height = 0;
}

switch (monitor_layout->orientation)
{
case ORIENTATION_LANDSCAPE:
case ORIENTATION_PORTRAIT:
case ORIENTATION_LANDSCAPE_FLIPPED:
case ORIENTATION_PORTRAIT_FLIPPED:
break;
default:
LOG(LOG_LEVEL_WARNING, "sanitise_extended_monitor_attributes:"
" Orientation is not one of %d, %d, %d, or %d."
" Value was %d and ignored and set to default value of LANDSCAPE.",
ORIENTATION_LANDSCAPE,
ORIENTATION_PORTRAIT,
ORIENTATION_LANDSCAPE_FLIPPED,
ORIENTATION_PORTRAIT_FLIPPED,
monitor_layout->orientation);
monitor_layout->orientation = ORIENTATION_LANDSCAPE;
}

int check_desktop_scale_factor
= monitor_layout->desktop_scale_factor < 100
|| monitor_layout->desktop_scale_factor > 500;
if (check_desktop_scale_factor)
{
LOG(LOG_LEVEL_WARNING, "sanitise_extended_monitor_attributes:"
" desktop_scale_factor is not within valid range"
" of [100, 500]. Assuming 100. Value was: %d",
monitor_layout->desktop_scale_factor);
}

int check_device_scale_factor
= monitor_layout->device_scale_factor != 100
&& monitor_layout->device_scale_factor != 140
&& monitor_layout->device_scale_factor != 180;
if (check_device_scale_factor)
{
LOG(LOG_LEVEL_WARNING, "sanitise_extended_monitor_attributes:"
" device_scale_factor a valid value (One of 100, 140, 180)."
" Assuming 100. Value was: %d",
monitor_layout->device_scale_factor);
}

if (check_desktop_scale_factor || check_device_scale_factor)
{
monitor_layout->desktop_scale_factor = 100;
monitor_layout->device_scale_factor = 100;
}
}

/*****************************************************************************/
/*
Process a [MS-RDPBCGR] TS_UD_CS_MONITOR message.
Expand Down Expand Up @@ -1907,87 +1999,11 @@ libxrdp_process_monitor_stream(struct stream *s,

in_uint32_le(s, monitor_layout->physical_width);
in_uint32_le(s, monitor_layout->physical_height);

/* Per spec (2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT),
* if EITHER physical_width or physical_height are
* out of range, BOTH must be ignored.
*/
if (monitor_layout->physical_width > 10000
|| monitor_layout->physical_width < 10)
{
LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
" physical_width is not within valid range."
" Setting physical_width to 0mm,"
" Setting physical_height to 0mm,"
" physical_width was: %d",
monitor_layout->physical_width);
monitor_layout->physical_width = 0;
monitor_layout->physical_height = 0;
}

if (monitor_layout->physical_height > 10000
|| monitor_layout->physical_height < 10)
{
LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
" physical_height is not within valid range."
" Setting physical_width to 0mm,"
" Setting physical_height to 0mm,"
" physical_height was: %d",
monitor_layout->physical_height);
monitor_layout->physical_width = 0;
monitor_layout->physical_height = 0;
}

in_uint32_le(s, monitor_layout->orientation);
switch (monitor_layout->orientation)
{
case ORIENTATION_LANDSCAPE:
case ORIENTATION_PORTRAIT:
case ORIENTATION_LANDSCAPE_FLIPPED:
case ORIENTATION_PORTRAIT_FLIPPED:
break;
default:
LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
" Orientation is not one of %d, %d, %d, or %d."
" Value was %d and ignored and set to default value of LANDSCAPE.",
ORIENTATION_LANDSCAPE,
ORIENTATION_PORTRAIT,
ORIENTATION_LANDSCAPE_FLIPPED,
ORIENTATION_PORTRAIT_FLIPPED,
monitor_layout->orientation);
monitor_layout->orientation = ORIENTATION_LANDSCAPE;
}

in_uint32_le(s, monitor_layout->desktop_scale_factor);
int check_desktop_scale_factor
= monitor_layout->desktop_scale_factor < 100
|| monitor_layout->desktop_scale_factor > 500;
if (check_desktop_scale_factor)
{
LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
" desktop_scale_factor is not within valid range"
" of [100, 500]. Assuming 100. Value was: %d",
monitor_layout->desktop_scale_factor);
}

in_uint32_le(s, monitor_layout->device_scale_factor);
int check_device_scale_factor
= monitor_layout->device_scale_factor != 100
&& monitor_layout->device_scale_factor != 140
&& monitor_layout->device_scale_factor != 180;
if (check_device_scale_factor)
{
LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
" device_scale_factor a valid value (One of 100, 140, 180)."
" Assuming 100. Value was: %d",
monitor_layout->device_scale_factor);
}

if (check_desktop_scale_factor || check_device_scale_factor)
{
monitor_layout->desktop_scale_factor = 100;
monitor_layout->device_scale_factor = 100;
}
sanitise_extended_monitor_attributes(monitor_layout);

/*
* 2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT
Expand Down Expand Up @@ -2137,5 +2153,108 @@ libxrdp_process_monitor_stream(struct stream *s,
monitor_layout->bottom =
monitor_layout->bottom - all_monitors_encompassing_bounds.top;
}

return 0;
}

/*****************************************************************************/
int
libxrdp_process_monitor_ex_stream(struct stream *s,
struct display_size_description *description)
{
uint32_t num_monitor;
uint32_t monitor_index;
uint32_t attribute_size;
struct monitor_info *monitor_layout;

LOG_DEVEL(LOG_LEVEL_TRACE, "libxrdp_process_monitor_ex_stream:");
if (description == NULL)
{
LOG_DEVEL(LOG_LEVEL_ERROR, "libxrdp_process_monitor_ex_stream: "
"description was null. "
" Valid pointer to allocated description expected.");
return SEC_PROCESS_MONITORS_ERR;
}

if (!s_check_rem_and_log(s, 4,
"libxrdp_process_monitor_ex_stream:"
" Parsing [MS-RDPBCGR] TS_UD_CS_MONITOR_EX"))
{
return SEC_PROCESS_MONITORS_ERR;
}

in_uint32_le(s, attribute_size);
if (attribute_size != TS_MONITOR_ATTRIBUTES_SIZE)
{
LOG(LOG_LEVEL_ERROR,
"libxrdp_process_monitor_ex_stream: [MS-RDPBCGR] Protocol"
" error: TS_UD_CS_MONITOR_EX monitorAttributeSize"
" MUST be %d, received: %d",
TS_MONITOR_ATTRIBUTES_SIZE, attribute_size);
return SEC_PROCESS_MONITORS_ERR;
}

in_uint32_le(s, num_monitor);
LOG(LOG_LEVEL_DEBUG, "libxrdp_process_monitor_ex_stream:"
" The number of monitors received is: %d",
num_monitor);

if (num_monitor != description->monitorCount)
{
LOG(LOG_LEVEL_ERROR,
"libxrdp_process_monitor_ex_stream: [MS-RDPBCGR] Protocol"
" error: TS_UD_CS_MONITOR monitorCount"
" MUST be %d, received: %d",
description->monitorCount, num_monitor);
return SEC_PROCESS_MONITORS_ERR;
}

for (monitor_index = 0; monitor_index < num_monitor; ++monitor_index)
{
if (!s_check_rem_and_log(s, attribute_size,
"libxrdp_process_monitor_ex_stream:"
" Parsing TS_UD_CS_MONITOR_EX"))
{
return SEC_PROCESS_MONITORS_ERR;
}

monitor_layout = description->minfo + monitor_index;

in_uint32_le(s, monitor_layout->physical_width);
in_uint32_le(s, monitor_layout->physical_height);
in_uint32_le(s, monitor_layout->orientation);
in_uint32_le(s, monitor_layout->desktop_scale_factor);
in_uint32_le(s, monitor_layout->device_scale_factor);

sanitise_extended_monitor_attributes(monitor_layout);

LOG_DEVEL(LOG_LEVEL_INFO, "libxrdp_process_monitor_ex_stream:"
" Received [MS-RDPBCGR] 2.2.1.3.9.1 "
" TS_MONITOR_ATTRIBUTES"
" Index: %d, PhysicalWidth %d, PhysicalHeight %d,"
" Orientation %d, DesktopScaleFactor %d,"
" DeviceScaleFactor %d",
monitor_index,
monitor_layout->physical_width,
monitor_layout->physical_height,
monitor_layout->orientation,
monitor_layout->desktop_scale_factor,
monitor_layout->device_scale_factor);
}

/* Update non negative monitor info values */
const struct monitor_info *src = description->minfo;
struct monitor_info *dst = description->minfo_wm;
for (monitor_index = 0; monitor_index < num_monitor; ++monitor_index)
{
dst->physical_width = src->physical_width;
dst->physical_height = src->physical_height;
dst->orientation = src->orientation;
dst->desktop_scale_factor = src->desktop_scale_factor;
dst->device_scale_factor = src->device_scale_factor;
++src;
++dst;
}

return 0;
}
3 changes: 2 additions & 1 deletion libxrdp/libxrdp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,9 +364,10 @@ xrdp_mcs_disconnect(struct xrdp_mcs *self);
/* xrdp_sec.c */

/*
These are error return codes for both:
These are error return codes for:-
1. xrdp_sec_process_mcs_data_monitors
2. libxrdp_process_monitor_stream
3. libxrdp_process_monitor_ex_stream
To clarify any reason for a non-zero response code.
*/
#define SEC_PROCESS_MONITORS_ERR 1
Expand Down
15 changes: 15 additions & 0 deletions libxrdp/libxrdpinc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,4 +324,19 @@ int EXPORT_CC
libxrdp_process_monitor_stream(struct stream *s, struct display_size_description *description,
int full_parameters);

/**
* Processes a stream that is based on [MS-RDPBCGR] 2.2.1.3.9 Client
* Monitor Extended Data (TS_UD_CS_MONITOR_EX)
*
* Data is stored in a struct which has already been read by
* libxrdp_process_monitor_stream() withj full_parameters set to 0.
*
* @param s Stream to read data from. Stream is read up to monitorAttributeSize
* @param description Result of reading TS_UD_CS_MONITOR PDU
* @return 0 if the data is processed, non-zero if there is an error.
*/
int EXPORT_CC
libxrdp_process_monitor_ex_stream(struct stream *s,
struct display_size_description *description);

#endif
Loading