[PR] Add OAuth2 HTTP request timeouts [pulsar-client-cpp]

["RobertIndie (via GitHub)" <gi...@apache.org>]

RobertIndie opened a new pull request, #606:
URL: https://github.com/apache/pulsar-client-cpp/pull/606

   
   <!--
   ### Contribution Checklist
     
     - PR title format should be *[type][component] summary*. For details, see *[Guideline - Pulsar PR Naming Convention](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#heading=h.trs9rsex3xom)*. 
   
     - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
     
     - Each pull request should address only one issue, not mix up code from multiple issues.
     
     - Each commit in the pull request has a meaningful commit message
   
     - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
   -->
   
   
   ### Motivation
   
   OAuth2 issuer discovery and token requests previously used libcurl without application-level connection or total request deadlines. An unavailable or stalled issuer could therefore delay client startup and recovery for several minutes.
   
   ### Modifications
   
   Add a distinct CurlWrapper connection timeout and configurable OAuth2 connect_timeout_seconds and request_timeout_seconds parameters, defaulting to 10 and 30 seconds. Validate both parameters as positive integers, apply them to discovery, token acquisition, and refresh for both OAuth2 flows, document the public configuration, and add black-box regression coverage.
   
   
   ### Verifying this change
   
   Built the modified production and AuthPluginTest objects with -Werror. Ran three new timeout and validation tests plus four existing OAuth TLS tests; all 7 passed. The complete pulsar-tests target remains blocked by the pre-existing DagWatchSession incompatibility with the installed Boost.Asio API.
   
   
   ### Usage
   
   Set connect_timeout_seconds and request_timeout_seconds as positive integer strings in the AuthOauth2 parameter map or JSON passed to AuthOauth2::create. If omitted, the client uses 10-second connection and 30-second total-request timeouts.
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc-required` 
   (Your PR needs to update docs and you will update later)
   
   - [ ] `doc-not-needed` 
   (Please explain why)
   
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Add OAuth2 HTTP request timeouts [pulsar-client-cpp]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #606:
URL: https://github.com/apache/pulsar-client-cpp/pull/606#discussion_r3747915067


##########
tests/AuthPluginTest.cc:
##########
@@ -678,6 +684,109 @@ static bool awaitMockServeResult(std::future<bool>& future, MockOauth2Server& se
 
 }  // namespace testOauth2Tls
 
+TEST(AuthPluginTest, testOauth2IssuerDiscoveryRequestTimeout) {
+    using testOauth2Tls::MockOauth2Server;
+
+    const std::string tokenBody = R"({"access_token":"mockToken","expires_in":3600,"token_type":"Bearer"})";
+    MockOauth2Server tokenServer(tokenBody, "application/json", 0, false);
+
+    std::ostringstream wellKnownBody;
+    wellKnownBody << R"({"token_endpoint":"https://localhost:)" << tokenServer.port() << R"(/oauth/token"})";
+    MockOauth2Server wellKnownServer(wellKnownBody.str(), "application/json", 0, false,
+                                     std::chrono::seconds(3));
+
+    std::thread tokenThread([&tokenServer]() { tokenServer.mockServe(); });
+    std::thread wellKnownThread([&wellKnownServer]() { wellKnownServer.mockServe(); });
+
+    ParamMap params;
+    params["issuer_url"] = "https://localhost:" + std::to_string(wellKnownServer.port());
+    params["client_id"] = "test-client";
+    params["client_secret"] = "test-secret";
+    params["connect_timeout_seconds"] = "1";
+    params["request_timeout_seconds"] = "1";
+
+    AuthenticationDataPtr data =
+        std::static_pointer_cast<AuthenticationDataProvider>(std::make_shared<InitialAuthData>(caPath));
+    AuthenticationPtr auth = AuthOauth2::create(params);
+
+    const auto start = std::chrono::steady_clock::now();
+    const Result result = auth->getAuthData(data);
+    const auto elapsedMs =
+        std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start)
+            .count();
+
+    wellKnownServer.stop();
+    tokenServer.stop();
+    wellKnownThread.join();
+    tokenThread.join();

Review Comment:
   These new timeout tests stop the mock servers and then join the server threads directly. If `mockServe()` is blocked (e.g., on `accept`, TLS handshake, or request read), the test can hang indefinitely; additionally, `mockServe()` does an unconditional `sleep_for(responseDelay_)`, so `stop()` won’t shorten the join wait. The existing OAuth2 TLS tests in this file use `std::promise/std::future` plus `awaitMockServeResult(...)` to ensure the mock threads always complete within `mockServerTimeout`; it would be safer/consistent to use the same pattern here (or make the delay interruptible via a stop flag/condition variable).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org