Skip to content

Commit 61cbf1a

Browse files
committed
Address comments
1 parent 116a1e8 commit 61cbf1a

4 files changed

Lines changed: 40 additions & 13 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ how much glue you want to write.
119119
capability runtime. One string generic per call gives request,
120120
response, and message types; a `web.json` allow-list is the public
121121
API surface; the same capabilities are also reachable over plain
122-
HTTP for `curl`, Postman, and AI-agent tool-use. _New in `2.0.0`._
122+
HTTP for `curl`, Postman, and AI-agent tool-use. _New in `2.0.0-beta.0`._
123123

124124
```ts
125125
import { connect } from 'rclnodejs/web';

demo/web/javascript/index.html

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -222,10 +222,17 @@ <h2>5. Same capability, no SDK — just <code>curl</code></h2>
222222
const host = location.hostname || 'localhost';
223223
const ENDPOINTS = {
224224
ws: `ws://${host}:9000/capability`,
225-
// HTTP base — the SDK derives the WS sibling automatically for
226-
// subscribe (see web/client.js _resolveUrls).
225+
// HTTP base for call/publish.
227226
http: `http://${host}:9001`,
228227
};
228+
// Pass Form C ({http, ws}) when the user picks HTTP so subscribe
229+
// still reaches the WS runtime on :9000. The SDK's auto-derived
230+
// sibling would land on :9001 instead and fail — this dev layout
231+
// splits the two transports across separate ports.
232+
const connectArg = (mode) =>
233+
mode === 'http'
234+
? { http: ENDPOINTS.http, ws: ENDPOINTS.ws }
235+
: ENDPOINTS.ws;
229236

230237
const endpointEl = document.getElementById('endpoint');
231238
const statusEl = document.getElementById('status');
@@ -236,7 +243,7 @@ <h2>5. Same capability, no SDK — just <code>curl</code></h2>
236243
const setEndpoint = (mode) => {
237244
endpointEl.textContent =
238245
mode === 'http'
239-
? `${ENDPOINTS.http} (subscribe lazily uses ws://${host}:9000/capability)`
246+
? `${ENDPOINTS.http} (subscribe routed to ${ENDPOINTS.ws})`
240247
: ENDPOINTS.ws;
241248
};
242249

@@ -270,14 +277,15 @@ <h2>5. Same capability, no SDK — just <code>curl</code></h2>
270277
setEndpoint(mode);
271278
setStatus(`connecting (${mode})…`);
272279
try {
273-
ros = await connect(ENDPOINTS[mode]);
280+
ros = await connect(connectArg(mode));
274281
setStatus(`connected (${mode})`, 'ok');
275282
} catch (e) {
276283
setStatus(`failed: ${e.message || e}`, 'err');
277284
return;
278285
}
279286
// Bind the always-on /web_demo_chatter subscription on every
280-
// reconnect. Over HTTP this lazily opens the WS sibling.
287+
// reconnect. Over HTTP the explicit { ws } in connectArg() makes
288+
// this work — see comment above.
281289
try {
282290
await ros.subscribe('/web_demo_chatter', (msg) =>
283291
log('chatLog', `<- ${msg.data}`),

demo/web/javascript/server.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ const http = require('http');
1919
const fs = require('fs');
2020

2121
const rclnodejs = require('../../../index.js');
22+
// In a downstream project this is the public, supported import:
23+
// const { createRuntime, WebSocketTransport, HttpTransport } =
24+
// require('rclnodejs/web/server');
25+
// Inside this in-repo demo we go through the relative path because the
26+
// `demo/web/javascript/` folder has its own package.json (so Node's
27+
// package self-reference doesn't see `rclnodejs` as resolvable here).
2228
const {
2329
createRuntime,
2430
WebSocketTransport,
@@ -128,7 +134,12 @@ async function main() {
128134
relPath = urlPath.replace(/^\/+/, '');
129135
}
130136
const filePath = path.resolve(baseDir, relPath);
131-
if (!filePath.startsWith(baseDir)) {
137+
// Confine reads to baseDir. `path.relative` collapses `..` segments,
138+
// so any escape attempt either yields a result that starts with `..`
139+
// or is absolute — reject both. (`startsWith(baseDir)` alone would
140+
// false-positive on a sibling like `${baseDir}-other/...`.)
141+
const rel = path.relative(baseDir, filePath);
142+
if (rel.startsWith('..') || path.isAbsolute(rel)) {
132143
res.writeHead(403).end('forbidden');
133144
return;
134145
}

demo/web/typescript/src/main.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,18 @@ type Mode = 'ws' | 'http';
2020
const HOST = location.hostname || 'localhost';
2121
const ENDPOINTS: Record<Mode, string> = {
2222
ws: `ws://${HOST}:9000/capability`,
23-
// HTTP base URL — the SDK derives the WS sibling automatically for
24-
// subscribe (see web/client.js _resolveUrls).
23+
// HTTP base for call/publish.
2524
http: `http://${HOST}:9001`,
2625
};
26+
// Pass Form C ({http, ws}) when the user picks HTTP so subscribe still
27+
// reaches the WS runtime on :9000. The SDK's auto-derived sibling would
28+
// land on :9001 instead and fail — this dev layout splits the two
29+
// transports across separate ports.
30+
function connectArg(mode: Mode): string | { http: string; ws: string } {
31+
return mode === 'http'
32+
? { http: ENDPOINTS.http, ws: ENDPOINTS.ws }
33+
: ENDPOINTS.ws;
34+
}
2735

2836
function $<T extends HTMLElement>(id: string): T {
2937
const el = document.getElementById(id);
@@ -40,7 +48,7 @@ function setStatus(text: string, cls: 'ok' | 'err' | '' = ''): void {
4048
function setEndpoint(mode: Mode): void {
4149
$('endpoint').textContent =
4250
mode === 'http'
43-
? `${ENDPOINTS.http} (subscribe lazily uses ws://${HOST}:9000/capability)`
51+
? `${ENDPOINTS.http} (subscribe routed to ${ENDPOINTS.ws})`
4452
: ENDPOINTS.ws;
4553
}
4654

@@ -83,15 +91,15 @@ async function main(): Promise<void> {
8391
setEndpoint(mode);
8492
setStatus(`connecting (${mode})…`);
8593
try {
86-
ros = await connect(ENDPOINTS[mode]);
94+
ros = await connect(connectArg(mode));
8795
setStatus(`connected (${mode})`, 'ok');
8896
} catch (e) {
8997
setStatus(`failed: ${String(e)}`, 'err');
9098
return;
9199
}
92100

93-
// Always-on chatter subscription; over HTTP this lazily opens
94-
// the WS sibling endpoint (subscribe always uses WS).
101+
// Always-on chatter subscription; subscribe always uses WS — the
102+
// explicit { ws } in connectArg() makes this work in HTTP mode too.
95103
try {
96104
await ros.subscribe<'std_msgs/msg/String'>('/web_demo_chatter', (msg) =>
97105
log('chatLog', `<- ${msg.data}`)

0 commit comments

Comments
 (0)