Skip to content

fix(server): drop raw user_data dump from McsMessage::SendDataRequest debug log#1295

Open
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:fix/server-mcs-log-noise
Open

fix(server): drop raw user_data dump from McsMessage::SendDataRequest debug log#1295
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:fix/server-mcs-log-noise

Conversation

@glamberson
Copy link
Copy Markdown
Contributor

Summary

  • debug!(?data, "McsMessage::SendDataRequest") invoked derived Debug on SendDataRequest<'a>, formatting user_data: Cow<'a, [u8]> as a decimal byte sequence.
  • At DEBUG level this produced multi-hundred-byte log lines per MCS request, including every keystroke / mouse event / channel message.
  • The raw bytes aren't actionable at this layer (channel processors log their own decoded PDUs); channel id, initiator, and payload length are.

Validation

  • cargo xtask check fmt/lints/tests/typos/locks all pass.
  • One log statement changed; no API or behavior change.

Notes

  • Per STYLE.md "Logging" section: structured fields are preferred over whole-struct ?data dumps.
  • user_data_len field name matches existing response_len precedent in ironrdp-async, ironrdp-acceptor, ironrdp-blocking.

… debug log

`debug!(?data, "McsMessage::SendDataRequest")` invoked the derived Debug
impl on `SendDataRequest<'a>`, which formats `user_data: Cow<'a, [u8]>`
as a decimal byte sequence. At DEBUG level the log line then ran to
several hundred bytes per MCS request -- and there is one MCS request
per keystroke, mouse event, channel message, etc. The raw bytes
themselves carry no diagnostic value at this layer (channel processors
log their own decoded PDUs); channel id, initiator, and payload length
are the actionable fields.

Switch to structured logging that preserves those three fields and
drops the payload. Single log statement, no API or behaviour change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant