Skip to content

Commit 47bf2ae

Browse files
chemicLchemicL
committed
feat!: guard required MCP spec fields against null
Records in McpSchema whose fields the MCP spec marks as required were storing them as nullable Java types with no validation. A caller constructing one of these records with null for a required field would silently produce invalid wire JSON because @JsonInclude(NON_ABSENT) omits null values without complaint. Compact canonical constructors with Assert.notNull are added to seven records: - JSONRPCError (code, message), - ProgressNotification (progressToken, progress), - LoggingMessageNotification (level, data), - CreateMessageRequest (messages, maxTokens), - CallToolResult (content), - SamplingMessage (role, content), - ElicitRequest (message, requestedSchema). Test code that constructed these records without required fields was updated to supply valid values; those tests were testing capability-check or error-path logic that fires after construction, so the missing fields were incidental rather than intentional. Signed-off-by: Dariusz Jędrzejczyk <2554306+chemicL@users.noreply.github.com>
1 parent d1cad25 commit 47bf2ae

5 files changed

Lines changed: 63 additions & 2 deletions

File tree

‎mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,11 @@ public record JSONRPCError( // @formatter:off
296296
@JsonProperty("code") Integer code,
297297
@JsonProperty("message") String message,
298298
@JsonProperty("data") Object data) { // @formatter:on
299+
300+
public JSONRPCError {
301+
Assert.notNull(code, "code must not be null");
302+
Assert.notNull(message, "message must not be null");
303+
}
299304
}
300305
}
301306

@@ -1573,6 +1578,10 @@ public record CallToolResult( // @formatter:off
15731578
@JsonProperty("structuredContent") Object structuredContent,
15741579
@JsonProperty("_meta") Map<String, Object> meta) implements Result { // @formatter:on
15751580

1581+
public CallToolResult {
1582+
Assert.notNull(content, "content must not be null");
1583+
}
1584+
15761585
/**
15771586
* Creates a builder for {@link CallToolResult}.
15781587
* @return a new builder instance
@@ -1797,6 +1806,11 @@ public static ModelHint of(String name) {
17971806
public record SamplingMessage( // @formatter:off
17981807
@JsonProperty("role") Role role,
17991808
@JsonProperty("content") Content content) { // @formatter:on
1809+
1810+
public SamplingMessage {
1811+
Assert.notNull(role, "role must not be null");
1812+
Assert.notNull(content, "content must not be null");
1813+
}
18001814
}
18011815

18021816
/**
@@ -1834,6 +1848,11 @@ public record CreateMessageRequest( // @formatter:off
18341848
@JsonProperty("metadata") Map<String, Object> metadata,
18351849
@JsonProperty("_meta") Map<String, Object> meta) implements Request { // @formatter:on
18361850

1851+
public CreateMessageRequest {
1852+
Assert.notNull(messages, "messages must not be null");
1853+
Assert.notNull(maxTokens, "maxTokens must not be null");
1854+
}
1855+
18371856
// backwards compatibility constructor
18381857
public CreateMessageRequest(List<SamplingMessage> messages, ModelPreferences modelPreferences,
18391858
String systemPrompt, ContextInclusionStrategy includeContext, Double temperature, Integer maxTokens,
@@ -2063,6 +2082,11 @@ public record ElicitRequest( // @formatter:off
20632082
@JsonProperty("requestedSchema") Map<String, Object> requestedSchema,
20642083
@JsonProperty("_meta") Map<String, Object> meta) implements Request { // @formatter:on
20652084

2085+
public ElicitRequest {
2086+
Assert.notNull(message, "message must not be null");
2087+
Assert.notNull(requestedSchema, "requestedSchema must not be null");
2088+
}
2089+
20662090
// backwards compatibility constructor
20672091
public ElicitRequest(String message, Map<String, Object> requestedSchema) {
20682092
this(message, requestedSchema, null);
@@ -2239,6 +2263,11 @@ public record ProgressNotification( // @formatter:off
22392263
@JsonProperty("message") String message,
22402264
@JsonProperty("_meta") Map<String, Object> meta) implements Notification { // @formatter:on
22412265

2266+
public ProgressNotification {
2267+
Assert.notNull(progressToken, "progressToken must not be null");
2268+
Assert.notNull(progress, "progress must not be null");
2269+
}
2270+
22422271
public ProgressNotification(Object progressToken, double progress, Double total, String message) {
22432272
this(progressToken, progress, total, message, null);
22442273
}
@@ -2281,6 +2310,11 @@ public record LoggingMessageNotification( // @formatter:off
22812310
@JsonProperty("data") String data,
22822311
@JsonProperty("_meta") Map<String, Object> meta) implements Notification { // @formatter:on
22832312

2313+
public LoggingMessageNotification {
2314+
Assert.notNull(level, "level must not be null");
2315+
Assert.notNull(data, "data must not be null");
2316+
}
2317+
22842318
// backwards compatibility constructor
22852319
public LoggingMessageNotification(LoggingLevel level, String logger, String data) {
22862320
this(level, logger, data, null);

‎mcp-core/src/test/java/io/modelcontextprotocol/server/McpAsyncServerExchangeTests.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ void testCreateElicitationWithNullCapabilities() {
306306

307307
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
308308
.message("Please provide your name")
309+
.requestedSchema(Map.of("type", "object"))
309310
.build();
310311

311312
StepVerifier.create(exchangeWithNullCapabilities.createElicitation(elicitRequest))
@@ -330,6 +331,7 @@ void testCreateElicitationWithoutElicitationCapabilities() {
330331

331332
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
332333
.message("Please provide your name")
334+
.requestedSchema(Map.of("type", "object"))
333335
.build();
334336

335337
StepVerifier.create(exchangeWithoutElicitation.createElicitation(elicitRequest)).verifyErrorSatisfies(error -> {
@@ -397,6 +399,7 @@ void testCreateElicitationWithDeclineAction() {
397399

398400
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
399401
.message("Please provide sensitive information")
402+
.requestedSchema(Map.of("type", "object"))
400403
.build();
401404

402405
McpSchema.ElicitResult expectedResult = McpSchema.ElicitResult.builder()
@@ -424,6 +427,7 @@ void testCreateElicitationWithCancelAction() {
424427

425428
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
426429
.message("Please provide your information")
430+
.requestedSchema(Map.of("type", "object"))
427431
.build();
428432

429433
McpSchema.ElicitResult expectedResult = McpSchema.ElicitResult.builder()
@@ -451,6 +455,7 @@ void testCreateElicitationWithSessionError() {
451455

452456
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
453457
.message("Please provide your name")
458+
.requestedSchema(Map.of("type", "object"))
454459
.build();
455460

456461
when(mockSession.sendRequest(eq(McpSchema.METHOD_ELICITATION_CREATE), eq(elicitRequest), any(TypeRef.class)))
@@ -474,6 +479,7 @@ void testCreateMessageWithNullCapabilities() {
474479
McpSchema.CreateMessageRequest createMessageRequest = McpSchema.CreateMessageRequest.builder()
475480
.messages(Arrays
476481
.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER, new McpSchema.TextContent("Hello, world!"))))
482+
.maxTokens(1000)
477483
.build();
478484

479485
StepVerifier.create(exchangeWithNullCapabilities.createMessage(createMessageRequest))
@@ -500,6 +506,7 @@ void testCreateMessageWithoutSamplingCapabilities() {
500506
McpSchema.CreateMessageRequest createMessageRequest = McpSchema.CreateMessageRequest.builder()
501507
.messages(Arrays
502508
.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER, new McpSchema.TextContent("Hello, world!"))))
509+
.maxTokens(1000)
503510
.build();
504511

505512
StepVerifier.create(exchangeWithoutSampling.createMessage(createMessageRequest)).verifyErrorSatisfies(error -> {
@@ -525,6 +532,7 @@ void testCreateMessageWithBasicRequest() {
525532
McpSchema.CreateMessageRequest createMessageRequest = McpSchema.CreateMessageRequest.builder()
526533
.messages(Arrays
527534
.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER, new McpSchema.TextContent("Hello, world!"))))
535+
.maxTokens(1000)
528536
.build();
529537

530538
McpSchema.CreateMessageResult expectedResult = McpSchema.CreateMessageResult.builder()
@@ -563,6 +571,7 @@ void testCreateMessageWithImageContent() {
563571
.messages(Arrays.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER,
564572
new McpSchema.ImageContent(null, "data:image/jpeg;base64,/9j/4AAQSkZJRgABAQEAYABgAAD...",
565573
"image/jpeg"))))
574+
.maxTokens(1000)
566575
.build();
567576

568577
McpSchema.CreateMessageResult expectedResult = McpSchema.CreateMessageResult.builder()
@@ -596,6 +605,7 @@ void testCreateMessageWithSessionError() {
596605
McpSchema.CreateMessageRequest createMessageRequest = McpSchema.CreateMessageRequest.builder()
597606
.messages(Arrays
598607
.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER, new McpSchema.TextContent("Hello"))))
608+
.maxTokens(1000)
599609
.build();
600610

601611
when(mockSession.sendRequest(eq(McpSchema.METHOD_SAMPLING_CREATE_MESSAGE), eq(createMessageRequest),
@@ -621,6 +631,7 @@ void testCreateMessageWithIncludeContext() {
621631
.messages(Arrays.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER,
622632
new McpSchema.TextContent("What files are available?"))))
623633
.includeContext(McpSchema.CreateMessageRequest.ContextInclusionStrategy.ALL_SERVERS)
634+
.maxTokens(1000)
624635
.build();
625636

626637
McpSchema.CreateMessageResult expectedResult = McpSchema.CreateMessageResult.builder()

‎mcp-core/src/test/java/io/modelcontextprotocol/server/McpSyncServerExchangeTests.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ void testCreateElicitationWithNullCapabilities() {
303303

304304
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
305305
.message("Please provide your name")
306+
.requestedSchema(Map.of("type", "object"))
306307
.build();
307308

308309
assertThatThrownBy(() -> exchangeWithNullCapabilities.createElicitation(elicitRequest))
@@ -326,6 +327,7 @@ void testCreateElicitationWithoutElicitationCapabilities() {
326327

327328
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
328329
.message("Please provide your name")
330+
.requestedSchema(Map.of("type", "object"))
329331
.build();
330332

331333
assertThatThrownBy(() -> exchangeWithoutElicitation.createElicitation(elicitRequest))
@@ -394,6 +396,7 @@ void testCreateElicitationWithDeclineAction() {
394396

395397
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
396398
.message("Please provide sensitive information")
399+
.requestedSchema(Map.of("type", "object"))
397400
.build();
398401

399402
McpSchema.ElicitResult expectedResult = McpSchema.ElicitResult.builder()
@@ -422,6 +425,7 @@ void testCreateElicitationWithCancelAction() {
422425

423426
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
424427
.message("Please provide your information")
428+
.requestedSchema(Map.of("type", "object"))
425429
.build();
426430

427431
McpSchema.ElicitResult expectedResult = McpSchema.ElicitResult.builder()
@@ -450,6 +454,7 @@ void testCreateElicitationWithSessionError() {
450454

451455
McpSchema.ElicitRequest elicitRequest = McpSchema.ElicitRequest.builder()
452456
.message("Please provide your name")
457+
.requestedSchema(Map.of("type", "object"))
453458
.build();
454459

455460
when(mockSession.sendRequest(eq(McpSchema.METHOD_ELICITATION_CREATE), eq(elicitRequest), any(TypeRef.class)))
@@ -475,6 +480,7 @@ void testCreateMessageWithNullCapabilities() {
475480
McpSchema.CreateMessageRequest createMessageRequest = McpSchema.CreateMessageRequest.builder()
476481
.messages(Arrays
477482
.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER, new McpSchema.TextContent("Hello, world!"))))
483+
.maxTokens(1000)
478484
.build();
479485

480486
assertThatThrownBy(() -> exchangeWithNullCapabilities.createMessage(createMessageRequest))
@@ -500,6 +506,7 @@ void testCreateMessageWithoutSamplingCapabilities() {
500506
McpSchema.CreateMessageRequest createMessageRequest = McpSchema.CreateMessageRequest.builder()
501507
.messages(Arrays
502508
.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER, new McpSchema.TextContent("Hello, world!"))))
509+
.maxTokens(1000)
503510
.build();
504511

505512
assertThatThrownBy(() -> exchangeWithoutSampling.createMessage(createMessageRequest))
@@ -525,6 +532,7 @@ void testCreateMessageWithBasicRequest() {
525532
McpSchema.CreateMessageRequest createMessageRequest = McpSchema.CreateMessageRequest.builder()
526533
.messages(Arrays
527534
.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER, new McpSchema.TextContent("Hello, world!"))))
535+
.maxTokens(1000)
528536
.build();
529537

530538
McpSchema.CreateMessageResult expectedResult = McpSchema.CreateMessageResult.builder()
@@ -564,6 +572,7 @@ void testCreateMessageWithImageContent() {
564572
.messages(Arrays.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER,
565573
new McpSchema.ImageContent(null, "data:image/jpeg;base64,/9j/4AAQSkZJRgABAQEAYABgAAD...",
566574
"image/jpeg"))))
575+
.maxTokens(1000)
567576
.build();
568577

569578
McpSchema.CreateMessageResult expectedResult = McpSchema.CreateMessageResult.builder()
@@ -598,6 +607,7 @@ void testCreateMessageWithSessionError() {
598607
McpSchema.CreateMessageRequest createMessageRequest = McpSchema.CreateMessageRequest.builder()
599608
.messages(Arrays
600609
.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER, new McpSchema.TextContent("Hello"))))
610+
.maxTokens(1000)
601611
.build();
602612

603613
when(mockSession.sendRequest(eq(McpSchema.METHOD_SAMPLING_CREATE_MESSAGE), eq(createMessageRequest),
@@ -624,6 +634,7 @@ void testCreateMessageWithIncludeContext() {
624634
.messages(Arrays.asList(new McpSchema.SamplingMessage(McpSchema.Role.USER,
625635
new McpSchema.TextContent("What files are available?"))))
626636
.includeContext(McpSchema.CreateMessageRequest.ContextInclusionStrategy.ALL_SERVERS)
637+
.maxTokens(1000)
627638
.build();
628639

629640
McpSchema.CreateMessageResult expectedResult = McpSchema.CreateMessageResult.builder()

‎mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ void testCreateMessageSuccess(String clientType) {
171171
.speedPriority(1.0)
172172
.intelligencePriority(1.0)
173173
.build())
174+
.maxTokens(1000)
174175
.build();
175176

176177
return exchange.createMessage(createMessageRequest)
@@ -250,6 +251,7 @@ void testCreateMessageWithRequestTimeoutSuccess(String clientType) throws Interr
250251
.speedPriority(1.0)
251252
.intelligencePriority(1.0)
252253
.build())
254+
.maxTokens(1000)
253255
.build();
254256

255257
return exchange.createMessage(createMessageRequest)
@@ -325,6 +327,7 @@ void testCreateMessageWithRequestTimeoutFail(String clientType) throws Interrupt
325327
.speedPriority(1.0)
326328
.intelligencePriority(1.0)
327329
.build())
330+
.maxTokens(1000)
328331
.build();
329332

330333
return exchange.createMessage(createMessageRequest).thenReturn(callResponse);

‎mcp-test/src/test/java/io/modelcontextprotocol/spec/McpSchemaTests.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,6 +1427,7 @@ void testCreateMessageResultUnknownStopReason() throws Exception {
14271427
@Test
14281428
void testCreateElicitationRequest() throws Exception {
14291429
McpSchema.ElicitRequest request = McpSchema.ElicitRequest.builder()
1430+
.message("Please provide additional information")
14301431
.requestedSchema(Map.of("type", "object", "required", List.of("a"), "properties",
14311432
Map.of("foo", Map.of("type", "string"))))
14321433
.build();
@@ -1436,8 +1437,9 @@ void testCreateElicitationRequest() throws Exception {
14361437
assertThatJson(value).when(Option.IGNORING_ARRAY_ORDER)
14371438
.when(Option.IGNORING_EXTRA_ARRAY_ITEMS)
14381439
.isObject()
1439-
.isEqualTo(json("""
1440-
{"requestedSchema":{"properties":{"foo":{"type":"string"}},"required":["a"],"type":"object"}}"""));
1440+
.isEqualTo(
1441+
json("""
1442+
{"message":"Please provide additional information","requestedSchema":{"properties":{"foo":{"type":"string"}},"required":["a"],"type":"object"}}"""));
14411443
}
14421444

14431445
@Test

0 commit comments

Comments
 (0)