Skip to content

THRIFT-5931: c_glib: avoid fixed-size buffers in thrift_ssl_socket_ge…#3355

Merged
Jens-G merged 1 commit intoapache:masterfrom
neosys007:codex/thrift-5931-ssl-error-formatting
Mar 22, 2026
Merged

THRIFT-5931: c_glib: avoid fixed-size buffers in thrift_ssl_socket_ge…#3355
Jens-G merged 1 commit intoapache:masterfrom
neosys007:codex/thrift-5931-ssl-error-formatting

Conversation

@neosys007
Copy link
Contributor

@neosys007 neosys007 commented Mar 21, 2026

This PR tightens the SSL error formatting path in the C GLib transport.

Current head still uses a fixed local char buffer[1024] plus a remaining-size counter that is updated by subtracting snprintf() return values. That pattern is fragile: once the formatted error text grows enough, the counter can underflow and the next append can compute a write position past the intended stack buffer.

The fix replaces that ad hoc buffer management with GString. That keeps the error message assembly simple, avoids remaining-space underflow, and removes the risk of walking past the fixed buffer while formatting chained SSL errors.

I kept the change isolated to thrift_ssl_socket_get_ssl_error() so the review only has to reason about one helper and its call sites.

Validation performed locally:

  • git diff --check
  • syntax-only compile for the changed C files
  • the file still shows pre-existing OpenSSL deprecation warnings in this tree, but there are no new compile errors from this change.

Related Jira:

…t_ssl_error()

thrift_ssl_socket_get_ssl_error() still builds SSL error messages in a fixed stack buffer while tracking remaining space with a signed counter that is updated by subtracting snprintf() return values.

If the formatted error text is long enough, that counter can underflow and the later writes can walk past the intended buffer boundaries.

Build the error message with GString instead so the helper no longer depends on hand-rolled remaining-space arithmetic.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
@mergeable mergeable bot added the c_glib label Mar 21, 2026
@Jens-G Jens-G merged commit 0de53c4 into apache:master Mar 22, 2026
80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants