Skip to content

Commit 6d04008

Browse files
fix(mcp): reverse validJson capability option and limit scope
1 parent b44ad38 commit 6d04008

File tree

3 files changed

+139
-3
lines changed

3 files changed

+139
-3
lines changed

packages/mcp-server/src/compat.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,11 @@ export function parseEmbeddedJSON(args: Record<string, unknown>, schema: Record<
7070
if (typeof value === 'string') {
7171
try {
7272
const parsed = JSON.parse(value);
73-
newArgs[key] = parsed;
74-
updated = true;
73+
// Only parse if result is a plain object (not array, null, or primitive)
74+
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
75+
newArgs[key] = parsed;
76+
updated = true;
77+
}
7578
} catch (e) {
7679
// Not valid JSON, leave as is
7780
}

packages/mcp-server/src/server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export async function executeHandler(
117117
compatibilityOptions?: Partial<ClientCapabilities>,
118118
) {
119119
const options = { ...defaultClientCapabilities, ...compatibilityOptions };
120-
if (options.validJson && args) {
120+
if (!options.validJson && args) {
121121
args = parseEmbeddedJSON(args, tool.inputSchema);
122122
}
123123
return await handler(client, args || {});

packages/mcp-server/tests/options.test.ts

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { parseOptions } from '../src/options';
22
import { Filter } from '../src/tools';
3+
import { parseEmbeddedJSON } from '../src/compat';
34

45
// Mock process.argv
56
const mockArgv = (args: string[]) => {
@@ -184,3 +185,135 @@ describe('parseOptions', () => {
184185
cleanup();
185186
});
186187
});
188+
189+
describe('parseEmbeddedJSON', () => {
190+
it('should not change non-string values', () => {
191+
const args = {
192+
numberProp: 42,
193+
booleanProp: true,
194+
objectProp: { nested: 'value' },
195+
arrayProp: [1, 2, 3],
196+
nullProp: null,
197+
undefinedProp: undefined,
198+
};
199+
const schema = {};
200+
201+
const result = parseEmbeddedJSON(args, schema);
202+
203+
expect(result).toBe(args); // Should return original object since no changes made
204+
expect(result['numberProp']).toBe(42);
205+
expect(result['booleanProp']).toBe(true);
206+
expect(result['objectProp']).toEqual({ nested: 'value' });
207+
expect(result['arrayProp']).toEqual([1, 2, 3]);
208+
expect(result['nullProp']).toBe(null);
209+
expect(result['undefinedProp']).toBe(undefined);
210+
});
211+
212+
it('should parse valid JSON objects in string properties', () => {
213+
const args = {
214+
jsonObjectString: '{"key": "value", "number": 123}',
215+
regularString: 'not json',
216+
};
217+
const schema = {};
218+
219+
const result = parseEmbeddedJSON(args, schema);
220+
221+
expect(result).not.toBe(args); // Should return new object since changes were made
222+
expect(result['jsonObjectString']).toEqual({ key: 'value', number: 123 });
223+
expect(result['regularString']).toBe('not json');
224+
});
225+
226+
it('should leave invalid JSON in string properties unchanged', () => {
227+
const args = {
228+
invalidJson1: '{"key": value}', // Missing quotes around value
229+
invalidJson2: '{key: "value"}', // Missing quotes around key
230+
invalidJson3: '{"key": "value",}', // Trailing comma
231+
invalidJson4: 'just a regular string',
232+
emptyString: '',
233+
};
234+
const schema = {};
235+
236+
const result = parseEmbeddedJSON(args, schema);
237+
238+
expect(result).toBe(args); // Should return original object since no changes made
239+
expect(result['invalidJson1']).toBe('{"key": value}');
240+
expect(result['invalidJson2']).toBe('{key: "value"}');
241+
expect(result['invalidJson3']).toBe('{"key": "value",}');
242+
expect(result['invalidJson4']).toBe('just a regular string');
243+
expect(result['emptyString']).toBe('');
244+
});
245+
246+
it('should not parse JSON primitives in string properties', () => {
247+
const args = {
248+
numberString: '123',
249+
floatString: '45.67',
250+
negativeNumberString: '-89',
251+
booleanTrueString: 'true',
252+
booleanFalseString: 'false',
253+
nullString: 'null',
254+
jsonArrayString: '[1, 2, 3, "test"]',
255+
regularString: 'not json',
256+
};
257+
const schema = {};
258+
259+
const result = parseEmbeddedJSON(args, schema);
260+
261+
expect(result).toBe(args); // Should return original object since no changes made
262+
expect(result['numberString']).toBe('123');
263+
expect(result['floatString']).toBe('45.67');
264+
expect(result['negativeNumberString']).toBe('-89');
265+
expect(result['booleanTrueString']).toBe('true');
266+
expect(result['booleanFalseString']).toBe('false');
267+
expect(result['nullString']).toBe('null');
268+
expect(result['jsonArrayString']).toBe('[1, 2, 3, "test"]');
269+
expect(result['regularString']).toBe('not json');
270+
});
271+
272+
it('should handle mixed valid objects and other JSON types', () => {
273+
const args = {
274+
validObject: '{"success": true}',
275+
invalidObject: '{"missing": quote}',
276+
validNumber: '42',
277+
validArray: '[1, 2, 3]',
278+
keepAsString: 'hello world',
279+
nonString: 123,
280+
};
281+
const schema = {};
282+
283+
const result = parseEmbeddedJSON(args, schema);
284+
285+
expect(result).not.toBe(args); // Should return new object since some changes were made
286+
expect(result['validObject']).toEqual({ success: true });
287+
expect(result['invalidObject']).toBe('{"missing": quote}');
288+
expect(result['validNumber']).toBe('42'); // Not parsed, remains string
289+
expect(result['validArray']).toBe('[1, 2, 3]'); // Not parsed, remains string
290+
expect(result['keepAsString']).toBe('hello world');
291+
expect(result['nonString']).toBe(123);
292+
});
293+
294+
it('should return original object when no strings are present', () => {
295+
const args = {
296+
number: 42,
297+
boolean: true,
298+
object: { key: 'value' },
299+
};
300+
const schema = {};
301+
302+
const result = parseEmbeddedJSON(args, schema);
303+
304+
expect(result).toBe(args); // Should return original object since no changes made
305+
});
306+
307+
it('should return original object when all strings are invalid JSON', () => {
308+
const args = {
309+
string1: 'hello',
310+
string2: 'world',
311+
string3: 'not json at all',
312+
};
313+
const schema = {};
314+
315+
const result = parseEmbeddedJSON(args, schema);
316+
317+
expect(result).toBe(args); // Should return original object since no changes made
318+
});
319+
});

0 commit comments

Comments
 (0)