[Don't merge] Build test#67
Open
hanatyan128 wants to merge 19 commits intomasterfrom
Open
Conversation
hanatyan128
force-pushed
the
feat-migrate-qt-network-to-curl-fix-2
branch
from
6dfde2e to
590243c
Compare
- Remove cancelAll(false) from OAuth2Client destructor to prevent canceling unrelated requests on shared CurlHttpClient - Add QPointer guards in OAuth2Client HTTP callbacks to prevent dangling this access after destruction - Change WsClient::connecting to std::atomic<bool> for thread safety - Implement exponential backoff (5s-300s) for WebSocket reconnection in both SRCLinkWebSocketClient and WsPortalClient - Reduce local HTTP server recv() timeout from 5s to 1s - Replace #define macros with static constexpr in CurlHttpClient, LocalHttpServer, and HttpRequestInvoker Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tighten libcurl/OAuth2/WebSocket plumbing against UAF cascades on plugin teardown, Bearer-token leakage via redirects, Linux CA path portability, nullptr-sender connect() pitfalls, and WsClient signal re-entry. Scope CMake CMP0077 inside FetchContent helpers, raise cmake_minimum_required to 3.22, request Qt6 6.6+, retire o2 references in docs, and bump Linux CI to ubuntu-24.04.
Replace Qt::SingleShotConnection (Qt 6.6+) usages in http-request-invoker.cpp with a connectQueuedOneShot() helper backed by std::shared_ptr<QMetaObject::Connection>, which disconnects after the first invocation on Qt 6.2. obs-deps 2024-03-19 ships Qt 6.6.2 for macOS / Windows but no Linux Qt6 asset, so Linux CI gets Qt from APT (Ubuntu 22.04 default = Qt 6.2.4). Reverting find_package(Qt6 6.6 REQUIRED) and the ubuntu-24.04 bump avoids a forced LTS-generation change while keeping all platforms green.
Extract the Linux CA-bundle resolution into a shared linux-ca-locations helper consumed by curl-http-client and ws-client, and replace the hard-coded /etc/ssl/certs with the SRC_LINK_LINUX_CA_PATH CMake cache variable. The helper stat(2)s the configured path and routes it to CURLOPT_CAPATH or CURLOPT_CAINFO depending on whether it is a directory or a regular file, so distro layouts (Debian, RHEL, Alpine, etc.) only need a single -D override at configure time.
WsClient::close() uses htons(1000) to encode the WebSocket close status code in network byte order. The header was only included inside #ifdef _WIN32 (via <winsock2.h>), so non-Windows builds failed with "'htons' was not declared in this scope".
Set CMAKE_POSITION_INDEPENDENT_CODE=ON inside the curl and mbedtls FetchContent setup functions. Without this, GNU ld on Linux refuses to link osi-src-link.so against libcurl.a / libmbed*.a with errors such as "relocation R_X86_64_PC32 against symbol stdin@@GLIBC_2.2.5 ... recompile with -fPIC". The flag has no effect on Windows / macOS linkers but keeping it function-scoped avoids leaking the setting into other subprojects.
The FetchContent-built libcurl 8.12.1 / mbedtls were inheriting the
plugin's -Werror policy and silently picking up Homebrew dependencies
on the macOS runner. Inside _src_link_setup_curl() and
_src_link_setup_mbedtls():
- Strip -Werror per target via set_target_properties +
target_compile_options(-Wno-error) so AppleClang stops failing on
curl's SIZE_T_MAX uses (-Wambiguous-macro).
- Set CMAKE_POLICY_DEFAULT_CMP0077 / CMP0126 to NEW so they survive
curl's own cmake_minimum_required(VERSION 3.7...3.16) reset, letting
our function-local CURL_BROTLI / CURL_ZSTD overrides win against
curl_dependency_option()'s CACHE STRING.
- Block find_package() for every optional dependency we do not ship
(Brotli, Zstd, LibSSH2, LibSSH, GSS, NGHTTP2, NGTCP2, LibIDN2,
LibPSL) and force CURL_DISABLE_SMB / CURL_DISABLE_SMBS so curl does
not auto-enable scp/sftp/smb against Homebrew libssh2 (which left
the final link unable to resolve "library 'ssh2'") or link against
Homebrew brotli ("library 'brotlidec' not found").
- Build curl/mbedtls with -fPIC for the shared plugin module on Linux.
- WsClient: guard pollRecv emit re-entrancy, deleteLater the recvNotifier, cap fragment buffer at 16 MB with WS close 1009, make connectGeneration atomic, queue ping-failure close emit, derive Origin scheme from URL, assert FD_SETSIZE on POSIX select(), pin TLS 1.2 minimum - CurlHttpClient: assign ctx->easy inside createEasyHandle, null-guard the CURLINFO_PRIVATE lookup, add a size*nmemb overflow guard to headerCallback, pin TLS 1.2 minimum - HttpRequestInvoker: privatize the queue() member template, narrow the sequencer drain disconnect so OperationCanceled still reaches consumers, switch the queue chain to Qt::QueuedConnection - SRCLinkWebSocketClient / WsPortalClient: hold the reconnect timer as a member so stop()/start()/destroyWsSocket() can cancel it, and align the backoff comment with the actual schedule - OAuth2Client: enforce the refresh thread contract via a runtime check that survives release builds - LocalHttpServer: shutdown(SD_SEND/SHUT_WR) before close to avoid RST on the OAuth callback, raise listen backlog to 5, log setsockopt failures - plugin-main: document the curl_global_init OBS-precondition; register obs_data_t* metatype during obs_module_load instead of post_load - ingress-link-source: replace a misleading thread-safety comment with a re-entrancy FIXME - CMakeLists: block ZLIB and CURL find_package, set CURL_ZLIB OFF, fail fast on missing mbedtls / libcurl_static targets, and compress the long policy / Windows-libs comments per the comment-discipline rule; FIXME the curl 8.12.1 pin rationale and bump plan - CLAUDE.md: document the macOS Xcode 15.2 CI pin, sync project version to 0.8.0
- WsClient: stop echoing peer-controlled CLOSE payload and reply with the
RFC 6455 1000 Normal Closure status; route auto-PONG through sendRaw so
send failures trigger cleanup and emit closed; pin TLS 1.2..1.3; align
fragment-buffer append cast with the surrounding qsizetype usage;
document open() silent-return contract and joinConnectThread() UI-thread
contract
- CurlHttpClient: raise the in-flight UI poll interval from 10 ms to 25 ms
to cut wakeup load while requests are active; add CURLOPT_TCP_KEEPALIVE
with 60s/30s idle/intvl so pooled idle connections don't silently
half-open across NATs; pin TLS 1.2..1.3
- HttpRequestSequencer / HttpRequestInvoker: leave a FIXME on the dtor's
synchronous emit+delete pair noting that the safer redesign is to
disconnect the invoker's outgoing finished->consumer connections
rather than the incoming chain; document the queue() invariant that
invoker() must not synchronously emit finished
- OAuth2Client: emit linkingFailed() from the re-entrancy guard in link()
so callers stop hanging on the 120s timeout; reject non-loopback values
in setLocalhostPolicy() per RFC 8252 §7.3 and re-validate at link()
entry
- LocalHttpServer: log a warning when the OAuth callback request fills
the 8 KiB buffer without terminating, and include WSAGetLastError /
errno in bind / listen / getsockname error messages
- linux-ca-locations: document the single-thread contract on
findLinuxCaLocation() in the header
- api-client: assert the httpClient parent invariant inside getPicture()
- plugin-main: tear down WsPortalEventHandler before deleting apiClient
during obs_module_unload, and FIXME the deeper dock-vs-apiClient
ordering for a separate PR
- CMakeLists: re-fetch ${mbedtls_SOURCE_DIR} via FetchContent_GetProperties
inside _src_link_setup_curl() so the include path resolves correctly
on Linux despite the function-scope reset
- run-cmake-format CI action: pin cmakelang to 0.6.13 to match the
clang-format 17.0.3 hard pin
- plugin-main: tear down EgressLinkDock / WsPortalDock before deleting apiClient so OBS-owned dock destructors do not run against freed WsPortalClient state - WsClient: drop CURLOPT_CONNECTTIMEOUT to 3s so OBS shutdown waits at most ~3s per worker; document the QPointer/joinConnectThread invariant on the worker -> UI invokeMethod; pin pingTimestamp to successful sends and gate pong RTT on a non-zero timestamp; log a partial fragment-buffer discard on close; include the port in the Origin header when it is not the scheme default; document the order invariant of cleanup() (notifier disconnect/deleteLater before curl_easy_cleanup); document sendRaw()'s UI-thread invariant; align the fragment-buffer cast with qsizetype - CurlHttpClient: pin allowed protocols to http,https on the easy handle and on redirected requests; reject CR/LF/NUL in header keys or values and fail the request via the existing nullptr path; classify headerCallback overflow as ResponseTooLarge to match writeCallback; FIXME the RequestContext raw-new pattern for a separate RAII PR - HttpRequestInvoker: in the 401 retry refresh-failure branch, remove self from the queue under the mutex before unlink/emit/delete to match the non-401 path and avoid re-entrant deadlock - SRCLinkApiClient: use OS ephemeral port for the OAuth callback listener; switch the cached-token freshness check to the static QDateTime::currentSecsSinceEpoch() form; FIXME the terminate() fire-and-forget DELETE that is canceled by plugin teardown - SRCLinkWebSocketClient / WsPortalClient: reset reconnect counter, timer, and pending flag on a successful connection so flap-then- recover restarts from the short backoff - LocalHttpServer: hard-fail listen on Windows when setsockopt(SO_EXCLUSIVEADDRUSE) is rejected to keep the OAuth callback off shared ports - IngressLinkSource: guard the reload_stages button callback with QPointer<IngressLinkSource> and a non-null source check before re-opening source properties; defer onSettingsUpdate's else-branch saveSettings() to the UI thread, matching the if-branch pattern - CMakeLists: replace the mbedtls fail-fast text with an explicit unsupported-host message; reject SRC_LINK_LINUX_CA_PATH values containing quote or backslash before they reach the C macro - build-aux/.run-format.zsh: enforce the cmake-format 0.6.13 upper bound locally to mirror the CI pin - CLAUDE.md: drop the deleted Frameworks.cmake.in entry and replace the old request-invoker reference with the src/net/ subtree
…quest invoker lifetime
…DME network stack notes
…giene - WsClient: reject CR/LF/NUL in upgrade headers; reply 1002 to peer CLOSE with reserved (1004/1005/1006/1015) or 1-byte payloads; close 1002 on PINGs > 125 bytes and accumulate split PING payloads before PONG echo; raise CONNECTTIMEOUT_MS back to 5000; replace assert(sockfd<FD_SETSIZE) with a release-safe runtime check; add CURLE_OK-with-zero-progress loop guard in sendRaw; route ping() through sendRaw with a 1-byte placeholder; QPointer-guard pollRecv re-entry and the queued errorOccurred/close pair; gate open() on connectThread.joinable() to avoid std::terminate; document idle-only setHeaders contract; add removePostedEvents in dtor - CurlHttpClient: drop CURLOPT_TCP_KEEPALIVE (easy handles are not reused); fail the request when curl_slist_append returns null; QPointer-guard self in checkCompleted so callback-driven destruction is safe; rework writeCallback/headerCallback bound checks to compare in size_t against remaining headroom - OAuth2Client: require a %1 placeholder in the localhost policy and re-validate at link() entry; drop the linkingFailed emit on re-entrant link() calls so the in-flight flow's terminal signal wins; defer the refreshFinished(AuthenticationRequired) emit when refresh() runs on a non-owner thread; clear pendingState_ before generating a new state - HttpRequestInvoker: in dtor, sever both directions of connections and call removePostedEvents to evict queued chain dispatches; document the mutex-after-requestQueue member-order invariant - LocalHttpServer: set SO_SNDTIMEO and loop send() until the full response is delivered; range-check response.size() against INT_MAX before narrowing - plugin-main: check curl_global_init return code; null-guard the settingsDialog show lambda; delete settingsDialog in obs_module_unload - IngressLinkSource: route the reload-stages connect() through guard.data(); log an INFO line when ~IngressLinkSource fires the fire-and-forget DELETE so the deferred cleanup is observable - api-client: log the same fire-and-forget DELETE caveat on terminate() - utils.hpp: raise GetAdaptersAddresses retry cap from 3 to 5 to absorb adapter-state churn between the size probe and data fetch - CMakeLists.txt: pin Qt6 floor to 6.2; relax the libcurl target check to require at least one of libcurl_static / libcurl_object; add rationale comments on the OS_* macro origin and Win32 deps - http-error.hpp: rewrite the Redirect FIXME in English - run-clang-format / run-cmake-format actions: pass --break-system-packages, log the resolved binary path, and anchor the version-pin grep
hanatyan128
force-pushed
the
feat-migrate-qt-network-to-curl-fix-2
branch
from
69dfd73 to
eb054a3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Build test only.