mRemotify/docker/patch-display-flush.py

#!/usr/bin/env python3
"""
Patch guacamole-server source files to fix bugs in git main that abort()
or segfault the child process in debug builds (no -DNDEBUG).

--- Fix 1: src/libguac/display-flush.c ---

The frame-flush loop iterates over display layers to diff pending vs last frame.
When a layer has pending_frame.buffer == NULL (e.g. cursor/aux layers that have
never been drawn), the code does:

    GUAC_ASSERT(current->pending_frame.buffer_is_external);
    continue;   <- BUG: never advances `current`

Two problems:
1. GUAC_ASSERT aborts the process if buffer_is_external is false, which happens
   for any fresh layer that has not yet received display data. In a debug build
   (no -DNDEBUG) this is a real assert -> abort() -> child process dies.
2. If the assert passes, `continue` loops forever on the same layer.

Fix: replace the GUAC_ASSERT with a proper pointer advance before the continue.

--- Fix 2: src/libguac/display-plan.c ---

Identical bug: the same GUAC_ASSERT(current->pending_frame.buffer_is_external)
pattern exists in display-plan.c (same layer-iteration loop, same missing pointer
advance before continue). Applying the same fix.

--- Fix 3: src/libguac/user.c ---

guac_user_supports_webp() iterates user->info.image_mimetypes without checking
whether the pointer itself is NULL. If the client did not send any image MIME
types (or guac_copy_mimetypes() returned NULL), the very first dereference
`*mimetype` causes a SIGSEGV in the display worker thread.

Fix: insert a NULL guard immediately after the pointer is loaded from the struct.

--- Fix 6: src/protocols/rdp/gdi.c (guac_rdp_gdi_end_paint) ---

With disable-gfx/legacy RDP, many Windows bitmap updates do NOT include
FrameMarker PDUs. guac_display_render_thread_notify_frame() (which wakes the
render thread to encode dirty tiles) is ONLY called from guac_rdp_gdi_mark_frame
— the FrameMarker handler. Legacy bitmaps that arrive without FrameMarkers mark
regions dirty but never wake the render thread → tiles are never encoded →
black squares on screen that only appear when a subsequent hover redraw (which
DOES carry a FrameMarker) finally wakes the thread for that region.

Root cause: the gdi_modified → notify_modified path in the main event loop only
fires AFTER the RDP event queue drains. During a continuous burst of bitmap
updates (initial desktop load), the queue never drains, so notify_modified is
never called, and the render thread sleeps indefinitely. Additionally, the render
thread applies lag compensation: if the browser hasn't ACK'd the last sync
instruction, processing_lag can reach 500 ms, and the render thread pauses up to
500 ms between flushes. Both factors combine to cause large areas of the screen
to remain black throughout the initial render.

Fix: call notify_frame unconditionally on every EndPaint. The render thread
naturally rate-limits output via MIN_FRAME_DURATION (10 ms = 100 fps max). To
prevent the processing_lag from reaching the 500 ms cap (which would cause the
render thread to throttle itself), the Node.js proxy (rdp.ts) immediately echoes
every sync instruction received from guacd back to guacd as a client ACK. This
keeps processing_lag near 0 ms so the render thread flushes as fast as
MIN_FRAME_DURATION allows. The browser's own sync ACKs are dropped by the proxy
to avoid double-updating the processing_lag measurement.
"""

import sys


def patch_file_replace(path, search_string, replacement_fn, description):
    """Replace the line containing search_string with replacement_fn(indent)."""
    with open(path) as f:
        lines = f.readlines()

    patched = False
    i = 0
    while i < len(lines):
        if search_string in lines[i]:
            indent = lines[i][: len(lines[i]) - len(lines[i].lstrip())]
            lines[i] = indent + replacement_fn(indent) + "\n"
            patched = True
            break
        i += 1

    if not patched:
        print("ERROR: patch target not found in " + path + " (" + description + ")", file=sys.stderr)
        sys.exit(1)

    with open(path, "w") as f:
        f.writelines(lines)

    print("Patched " + path + " (" + description + ")")


def patch_file_insert_after(path, search_string, insert_lines, description):
    """Insert insert_lines (with matching indent) after the line containing search_string."""
    with open(path) as f:
        lines = f.readlines()

    patched = False
    i = 0
    while i < len(lines):
        if search_string in lines[i]:
            indent = lines[i][: len(lines[i]) - len(lines[i].lstrip())]
            for j, new_line in enumerate(insert_lines):
                lines.insert(i + 1 + j, indent + new_line + "\n")
            patched = True
            break
        i += 1

    if not patched:
        print("ERROR: patch target not found in " + path + " (" + description + ")", file=sys.stderr)
        sys.exit(1)

    with open(path, "w") as f:
        f.writelines(lines)

    print("Patched " + path + " (" + description + ")")


# Fix 1: display-flush.c
patch_file_replace(
    "src/libguac/display-flush.c",
    "GUAC_ASSERT(current->pending_frame.buffer_is_external)",
    lambda indent: "current = current->pending_frame.next;",
    "replace GUAC_ASSERT with pointer advance",
)

# Fix 2: display-plan.c (identical bug pattern)
patch_file_replace(
    "src/libguac/display-plan.c",
    "GUAC_ASSERT(current->pending_frame.buffer_is_external)",
    lambda indent: "current = current->pending_frame.next;",
    "replace GUAC_ASSERT with pointer advance",
)

# Fix 3: gdi.c - handle begin_paint context still open during desktop_resize.
# With the GFX pipeline on FreeRDP < 3.8.0, begin_paint may be active when
# desktop_resize fires, leaving current_context non-NULL. FreeRDP 3.8.0 fixed
# this by calling EndPaint inside gdi_resize(); backport that behaviour here.
gdi_path = "src/protocols/rdp/gdi.c"
with open(gdi_path) as f:
    gdi_content = f.read()

gdi_old = (
    "    /* For FreeRDP versions prior to 3.8.0, EndPaint will not be called in\n"
    "     * `gdi_resize()`, so the current context should be NULL. If it is not\n"
    "     * NULL, it means that the current context is still open, and therefore the\n"
    "     * GDI buffer has not been flushed yet. */\n"
    "    GUAC_ASSERT(rdp_client->current_context == NULL);"
)

gdi_new = (
    "    /* For FreeRDP < 3.8.0, gdi_resize() does not call EndPaint internally.\n"
    "     * With the GFX pipeline begin_paint may still be active when desktop_resize\n"
    "     * fires, leaving current_context non-NULL. End the paint now so the context\n"
    "     * is properly closed before we open a new one for the resize. */\n"
    "    if (rdp_client->current_context != NULL)\n"
    "        guac_rdp_gdi_end_paint(context);"
)

if gdi_old not in gdi_content:
    print("ERROR: patch target not found in " + gdi_path + " (desktop_resize assert)", file=sys.stderr)
    sys.exit(1)

gdi_content = gdi_content.replace(gdi_old, gdi_new, 1)
with open(gdi_path, "w") as f:
    f.write(gdi_content)

print("Patched " + gdi_path + " (desktop_resize assert -> end_paint guard)")


# Fix 5: user.c - NULL guard for image_mimetypes in guac_user_supports_webp
patch_file_insert_after(
    "src/libguac/user.c",
    "const char** mimetype = user->info.image_mimetypes;",
    [
        "",
        "/* Guard: image_mimetypes may be NULL if client sent no image types */",
        "if (mimetype == NULL)",
        "    return 0;",
        "",
    ],
    "NULL guard for image_mimetypes in guac_user_supports_webp",
)

# Fix 6: gdi.c - notify_frame on every EndPaint (no rate limit)
#
# Problem: legacy RDP bitmap-cache updates (taskbar icons, static content) carry
# no FrameMarker PDU. The gdi_modified → notify_modified path only fires after
# the RDP event queue drains — never during a burst — so tiles accumulate as
# black squares. Additionally the render thread's lag-compensation can pause up
# to 500 ms between flushes if the browser hasn't ACK'd the previous sync.
#
# Fix: call notify_frame unconditionally in EndPaint. The render thread's own
# MIN_FRAME_DURATION (10 ms) provides natural batching at up to 100 fps. The
# sync-flood / lag-compensation problem is solved at the proxy layer (rdp.ts):
# the Node.js proxy immediately echoes every sync timestamp back to guacd so
# processing_lag stays near 0 ms, and the browser's own ACKs are dropped to
# prevent double-updating the measurement.
gdi_end_paint_old = (
    "    /* There will be no further drawing operations */\n"
    "    rdp_client->current_context = NULL;\n"
    "    guac_display_layer_close_raw(default_layer, current_context);\n"
    "\n"
    "    return TRUE;\n"
)

gdi_end_paint_new = (
    "    /* There will be no further drawing operations */\n"
    "    rdp_client->current_context = NULL;\n"
    "    guac_display_layer_close_raw(default_layer, current_context);\n"
    "\n"
    "    /* Notify the render thread that new tiles are ready to encode.\n"
    "     * notify_frame rather than notify_modified ensures the frame counter\n"
    "     * advances and a sync is emitted after each batch. This causes legacy\n"
    "     * bitmap-cache updates (which carry no FrameMarker PDU) to be flushed\n"
    "     * immediately on every EndPaint instead of accumulating as black squares\n"
    "     * until the next FrameMarker. The render thread's MIN_FRAME_DURATION\n"
    "     * (10 ms) limits output to 100 fps; the Node.js proxy handles sync ACKs\n"
    "     * so guacd's processing_lag stays near 0 ms. */\n"
    "    if (rdp_client->render_thread != NULL)\n"
    "        guac_display_render_thread_notify_frame(rdp_client->render_thread);\n"
    "\n"
    "    return TRUE;\n"
)

gdi_path = "src/protocols/rdp/gdi.c"
with open(gdi_path) as f:
    gdi_end_content = f.read()

if gdi_end_paint_old not in gdi_end_content:
    print("ERROR: patch target not found in " + gdi_path + " (notify_frame on EndPaint)", file=sys.stderr)
    sys.exit(1)

gdi_end_content = gdi_end_content.replace(gdi_end_paint_old, gdi_end_paint_new, 1)
with open(gdi_path, "w") as f:
    f.write(gdi_end_content)

print("Patched " + gdi_path + " (notify_frame on every EndPaint)")