Skip to content

Commit ac27f93

Browse files
ai: apply changes for #876 (1 review thread)
Addresses: - #3636101139 at tests/unit/test_auth.py:210 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 121bd0b commit ac27f93

1 file changed

Lines changed: 88 additions & 0 deletions

File tree

tests/unit/test_auth.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,94 @@ def run():
277277
str(oauth_manager._redirect_callback_timeout_seconds), error_message
278278
)
279279

280+
@patch("databricks.sql.auth.oauth.webbrowser.open_new")
281+
@patch.object(OAuthManager, "_OAuthManager__fetch_well_known_config")
282+
def test_get_tokens_does_not_hang_when_callback_connects_but_never_completes(
283+
self, mock_fetch_config, mock_open_new
284+
):
285+
"""A client that connects to the local redirect server but never sends a
286+
complete HTTP request line must not block forever either. This is the
287+
separate defense the PR adds via ``handler.timeout``: setup() applies it
288+
to the accepted socket, so the stalled request-line read raises
289+
socket.timeout, which socketserver swallows via handle_error. Because no
290+
callback path is ever recorded (request_path stays None) and this is not
291+
the accept-wait timeout (callback_timed_out stays False), the flow raises
292+
the generic "No path parameters" error rather than the "Timed out" one.
293+
See oauth.py __get_authorization_code."""
294+
import socket
295+
import threading
296+
from urllib.parse import urlparse, parse_qs
297+
298+
mock_fetch_config.return_value = {
299+
"authorization_endpoint": "https://foo.cloud.databricks.com/oidc/oauth2/v2.0/authorize",
300+
"token_endpoint": "https://foo.cloud.databricks.com/oidc/oauth2/v2.0/token",
301+
}
302+
303+
# Discover a currently-free port and target it explicitly. Unlike the
304+
# accept-wait test above, this one needs the connecting client to know
305+
# where to reach the redirect server; port_range=[0] would leave the
306+
# redirect URL pointing at port 0.
307+
probe = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
308+
probe.bind(("localhost", 0))
309+
free_port = probe.getsockname()[1]
310+
probe.close()
311+
312+
stalled_sockets = []
313+
314+
def connect_but_stall(auth_req_uri):
315+
# Instead of launching a browser, open a bare TCP connection to the
316+
# local redirect server (parsed out of the authorization request's
317+
# redirect_uri) and never send a request line, so the accepted
318+
# socket's read stalls until handler.timeout fires.
319+
redirect_uri = parse_qs(urlparse(auth_req_uri).query)["redirect_uri"][0]
320+
port = urlparse(redirect_uri).port
321+
stalled_sockets.append(socket.create_connection(("localhost", port)))
322+
return True
323+
324+
mock_open_new.side_effect = connect_but_stall
325+
326+
oauth_manager = OAuthManager(
327+
port_range=[free_port],
328+
client_id="mock-id",
329+
idp_endpoint=InHouseOAuthEndpointCollection(),
330+
http_client=MagicMock(),
331+
redirect_callback_timeout_seconds=2,
332+
)
333+
334+
result = {}
335+
336+
def run():
337+
try:
338+
oauth_manager.get_tokens(
339+
hostname="foo.cloud.databricks.com", scope="offline_access sql"
340+
)
341+
result["outcome"] = "returned"
342+
except BaseException as e: # noqa: BLE001
343+
result["outcome"] = "raised"
344+
result["error"] = e
345+
346+
worker = threading.Thread(target=run, daemon=True)
347+
worker.start()
348+
worker.join(timeout=30)
349+
350+
try:
351+
self.assertFalse(
352+
worker.is_alive(),
353+
"OAuth callback server blocked indefinitely on a client that "
354+
"connected but never completed the request line",
355+
)
356+
self.assertEqual(result.get("outcome"), "raised")
357+
self.assertIsInstance(result.get("error"), RuntimeError)
358+
# A stalled request-line read is NOT the accept-wait timeout:
359+
# request_path stays None and callback_timed_out stays False, so the
360+
# generic message is raised, not the "Timed out" one.
361+
error_message = str(result.get("error"))
362+
self.assertIn("No path parameters", error_message)
363+
self.assertNotIn("Timed out", error_message)
364+
finally:
365+
for sock in stalled_sockets:
366+
sock.close()
367+
280368

281369
class TestClientCredentialsTokenSource:
282370
@pytest.fixture

0 commit comments

Comments
 (0)