Skip to content

Commit 175dcb8

Browse files
fix: user-agent should be uri safe (#2546)
1 parent 700a636 commit 175dcb8

File tree

3 files changed

+106
-1
lines changed

3 files changed

+106
-1
lines changed

.changeset/safe-user-agent.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@slack/web-api": patch
3+
---
4+
5+
Fix user-agent header to URI-encode characters outside the Latin-1 range, preventing errors when `process.title` contains non-ASCII characters
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import assert from 'node:assert/strict';
2+
import { createRequire } from 'node:module';
3+
import { afterEach, describe, it } from 'node:test';
4+
5+
const require = createRequire(import.meta.url);
6+
7+
function isLatin1Safe(s: string): boolean {
8+
return Buffer.from(s, 'latin1').toString('latin1') === s;
9+
}
10+
11+
describe('instrument', () => {
12+
const originalDescriptor = Object.getOwnPropertyDescriptor(process, 'title');
13+
const modulePath = require.resolve('./instrument.ts');
14+
15+
afterEach(() => {
16+
// Restore the original process.title property
17+
if (originalDescriptor) {
18+
Object.defineProperty(process, 'title', originalDescriptor);
19+
}
20+
// Clear module cache so next require gets a fresh evaluation
21+
delete require.cache[modulePath];
22+
});
23+
24+
function mockProcessTitle(title: string): void {
25+
Object.defineProperty(process, 'title', {
26+
get: () => title,
27+
configurable: true,
28+
});
29+
}
30+
31+
/**
32+
* Returns a fresh import of the instrument module. Since `baseUserAgent` is computed at module
33+
* load time, we clear the require cache and re-require to pick up a mocked `process.title`.
34+
*/
35+
function freshImport(): typeof import('./instrument') {
36+
delete require.cache[modulePath];
37+
return require(modulePath);
38+
}
39+
40+
describe('getUserAgent', () => {
41+
it('should contain the package name', () => {
42+
const { getUserAgent } = freshImport();
43+
const ua = getUserAgent();
44+
assert.ok(ua.includes('@slack:web-api'), `User-Agent should contain @slack:web-api: ${ua}`);
45+
});
46+
47+
it('should include an ASCII process.title in the user agent', () => {
48+
mockProcessTitle('node');
49+
const { getUserAgent } = freshImport();
50+
const ua = getUserAgent();
51+
assert.ok(ua.includes('node/'), `User-Agent should contain node/: ${ua}`);
52+
assert.ok(isLatin1Safe(ua));
53+
});
54+
55+
it('should include other ASCII process.title in the user agent', () => {
56+
mockProcessTitle('openclaw-gateway');
57+
const { getUserAgent } = freshImport();
58+
const ua = getUserAgent();
59+
assert.ok(ua.includes('openclaw-gateway/'), `User-Agent should contain openclaw-gateway/: ${ua}`);
60+
assert.ok(isLatin1Safe(ua));
61+
});
62+
63+
it('should return a Latin-1 safe user agent when process.title contains non-Latin-1 characters', () => {
64+
const notLatin1SafeTitle = '管理者: Windows PowerShell';
65+
assert.strictEqual(isLatin1Safe(notLatin1SafeTitle), false);
66+
67+
mockProcessTitle(notLatin1SafeTitle);
68+
const { getUserAgent } = freshImport();
69+
const ua = getUserAgent();
70+
assert.ok(isLatin1Safe(ua), `User-Agent contains non-Latin-1 characters: ${ua}`);
71+
assert.ok(!ua.includes(notLatin1SafeTitle), 'User-Agent should not contain raw non-ASCII characters');
72+
assert.ok(
73+
ua.includes('%E7%AE%A1%E7%90%86%E8%80%85: Windows PowerShell'),
74+
'User-Agent should percent-encode only non-Latin-1 characters',
75+
);
76+
});
77+
78+
it('should preserve Latin-1 characters in process.title', () => {
79+
mockProcessTitle('café');
80+
const { getUserAgent } = freshImport();
81+
const ua = getUserAgent();
82+
assert.ok(ua.includes('café/'), `User-Agent should preserve Latin-1 characters: ${ua}`);
83+
assert.ok(isLatin1Safe(ua));
84+
});
85+
});
86+
});

packages/web-api/src/instrument.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,20 @@ function replaceSlashes(s: string): string {
1010
return s.replace('/', ':');
1111
}
1212

13+
const MAX_LATIN1_CODE = 0xff;
14+
15+
/**
16+
* Ensures a string is safe for use in HTTP headers by URI-encoding characters outside the Latin-1 (ISO-8859-1) range.
17+
* Latin-1 characters (code points 0x00–0xFF) are preserved as-is; all others are percent-encoded via encodeURIComponent.
18+
*/
19+
function toLatin1Safe(s: string): string {
20+
let result = '';
21+
for (const char of s) {
22+
result += char.charCodeAt(0) <= MAX_LATIN1_CODE ? char : encodeURIComponent(char);
23+
}
24+
return result;
25+
}
26+
1327
// TODO: for the deno build (see the `npm run build:deno` npm run script), we could replace the `os-browserify` npm
1428
// module shim with our own shim leveraging the deno beta compatibility layer for node's `os` module (for more info
1529
// see https://deno.land/std@0.116.0/node/os.ts). At the time of writing this TODO (2021/11/25), this required deno
@@ -18,7 +32,7 @@ function replaceSlashes(s: string): string {
1832
// based code will report "browser/undefined" from a deno runtime.
1933
const baseUserAgent =
2034
`${replaceSlashes(packageJson.name)}/${packageJson.version} ` +
21-
`${basename(process.title)}/${process.version.replace('v', '')} ` +
35+
`${toLatin1Safe(basename(process.title))}/${process.version.replace('v', '')} ` +
2236
`${os.platform()}/${os.release()}`;
2337

2438
const appMetadata: { [key: string]: string } = {};

0 commit comments

Comments
 (0)