Skip to content

fix(client-python): normalize server URI paths before appending task service #738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wdwd720 wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

fix(client-python): normalize server URI paths before appending task service #738

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
508664f
fix(client-python): normalize server URI paths
wdwd720 May 1, 2026
6b7d776
test(client-python): cover additional URI normalization schemes
wdwd720 May 1, 2026
4de158d
fix(client-typescript): normalize server URI paths before appending t…
Rod-Christensen May 1, 2026
77bb5c5
Merge branch 'develop' into fix/python-uri-normalization-paths
kwit75 May 1, 2026
540e254
Merge branch 'develop' into fix/python-uri-normalization-paths
kwit75 May 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/client-python/src/rocketride/mixins/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,11 @@ def _get_websocket_uri(uri: str) -> str:
parsed = urllib.parse.urlparse(normalized)

ws_scheme = 'wss' if parsed.scheme in ('https', 'wss') else 'ws'
ws_uri = parsed._replace(scheme=ws_scheme)
return f'{ws_uri.geturl()}/task/service'
path = parsed.path.rstrip('/')
if not path.endswith('/task/service'):
path = f'{path}/task/service' if path else '/task/service'
ws_uri = parsed._replace(scheme=ws_scheme, path=path)
return ws_uri.geturl()

def _set_uri(self, uri: str) -> None:
"""Update the server URI (internal)."""
Expand Down
22 changes: 22 additions & 0 deletions packages/client-python/tests/test_connection_uri.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import pytest

from rocketride.mixins.connection import ConnectionMixin


@pytest.mark.parametrize(
('input_uri', 'expected_uri'),
[
('http://localhost:5565', 'ws://localhost:5565/task/service'),
('http://localhost:5565/', 'ws://localhost:5565/task/service'),
('https://cloud.rocketride.ai', 'wss://cloud.rocketride.ai/task/service'),
('https://cloud.rocketride.ai/', 'wss://cloud.rocketride.ai/task/service'),
('wss://cloud.rocketride.ai', 'wss://cloud.rocketride.ai/task/service'),
('wss://cloud.rocketride.ai/', 'wss://cloud.rocketride.ai/task/service'),
('wss://cloud.rocketride.ai/task/service', 'wss://cloud.rocketride.ai/task/service'),
('ws://localhost:5565', 'ws://localhost:5565/task/service'),
('ws://localhost:5565/', 'ws://localhost:5565/task/service'),
('ws://localhost:5565/task/service', 'ws://localhost:5565/task/service'),
],
Comment thread
coderabbitai[bot] marked this conversation as resolved.
)
def test_get_websocket_uri_normalizes_task_service_path(input_uri, expected_uri):
assert ConnectionMixin._get_websocket_uri(input_uri) == expected_uri
14 changes: 12 additions & 2 deletions packages/client-typescript/src/client/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,13 +435,23 @@ export class RocketRideClient extends DAPClient {
*/
private _getWebsocketUri(uri: string): string {
const httpUrl = RocketRideClient.normalizeUri(uri);
const SERVICE_PATH = '/task/service';

try {
const url = new URL(httpUrl);
const wsScheme = url.protocol === 'https:' || url.protocol === 'wss:' ? 'wss:' : 'ws:';
return `${wsScheme}//${url.host}/task/service`;
// Normalize the path: strip trailing slash, then append /task/service
// only if it isn't already present (prevents double-path when callers
// pass an already-normalized WebSocket endpoint).
let path = url.pathname.replace(/\/+$/, '');
if (!path.endsWith(SERVICE_PATH)) {
path = path ? `${path}${SERVICE_PATH}` : SERVICE_PATH;
}
return `${wsScheme}//${url.host}${path}`;
} catch {
return `${httpUrl}/task/service`;
// Fallback for unparseable URIs — same trailing-slash + dedup logic
const stripped = httpUrl.replace(/\/+$/, '');
return stripped.endsWith(SERVICE_PATH) ? stripped : `${stripped}${SERVICE_PATH}`;
Comment on lines +446 to +454
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether the TS test file already covers trailing-slash and pre-normalized-path cases.
rg -n "trailing|task/service" --type=ts packages/client-typescript/tests/

Repository: rocketride-org/rocketride-server

Length of output: 608

🏁 Script executed:

# First, read the actual test file context around the URI normalization tests
sed -n '2186,2200p' packages/client-typescript/tests/RocketRideClient.test.ts

Repository: rocketride-org/rocketride-server

Length of output: 681

🏁 Script executed:

# Read the code being tested to verify the logic matches the suggested test cases
sed -n '440,460p' packages/client-typescript/src/client/client.ts

Repository: rocketride-org/rocketride-server

Length of output: 880

TypeScript tests are missing the exact regression cases this PR fixes.

The existing RocketRideClient URI normalization suite (lines 2186-2198) covers only bare host inputs. The PR's stated fixes — preventing double trailing slashes (e.g., wss://host//task/service) and avoiding duplicate path segments (e.g., wss://host/task/service/task/service) — have no TS-side test coverage.

Suggested additions:

 it.each([
   ['wss://cloud.rocketride.ai', 'wss://cloud.rocketride.ai/task/service'],
   ['https://cloud.rocketride.ai', 'wss://cloud.rocketride.ai/task/service'],
   ['ws://localhost:5565', 'ws://localhost:5565/task/service'],
   ['http://localhost:5565', 'ws://localhost:5565/task/service'],
+  // trailing-slash regression: must not produce double slash
+  ['wss://cloud.rocketride.ai/', 'wss://cloud.rocketride.ai/task/service'],
+  ['https://cloud.rocketride.ai/', 'wss://cloud.rocketride.ai/task/service'],
+  ['http://localhost:5565/', 'ws://localhost:5565/task/service'],
+  // pre-normalized path: must not duplicate /task/service
+  ['wss://cloud.rocketride.ai/task/service', 'wss://cloud.rocketride.ai/task/service'],
+  ['https://cloud.rocketride.ai/task/service', 'wss://cloud.rocketride.ai/task/service'],
 ])('normalizes %s to %s', (inputUri, expectedUri) => {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/client-typescript/src/client/client.ts` around lines 446 - 454, Add
TypeScript tests to the existing "RocketRideClient URI normalization" suite to
cover the regression cases this PR fixes: add cases that ensure duplicate
slashes are collapsed (e.g., input "wss://host//task/service" => normalized
"wss://host/task/service") and cases that prevent duplicated path segments
(e.g., input "wss://host/task/service/task/service" => normalized
"wss://host/task/service"). Place these new assertions alongside the existing
RocketRideClient tests (the suite referenced in client.ts URI normalization
logic that uses SERVICE_PATH and the try/catch path normalization), verifying
both the parsed/wsScheme branch and the fallback unparseable-URI branch produce
the expected single canonical path.

}
}

Expand Down
Loading