Skip to content

Commit 84d7ce0

Browse files
committed
Prevent NullPointerException from null context values in ChatClient
Add validation to reject null values in the context map of ChatClientRequest and ChatClientResponse. This prevents NullPointerException when advisors use Map.copyOf() to copy the context, as Map.copyOf() does not allow null values. - Add null value validation in record constructors - Add null key and value validation in Builder.context(Map) - Add null value validation in Builder.context(String, Object) - Add corresponding tests for all new validations Closes gh-4952 Signed-off-by: Nhahan <[email protected]>
1 parent 89a3b32 commit 84d7ce0

File tree

4 files changed

+73
-5
lines changed

4 files changed

+73
-5
lines changed

spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/ChatClientRequest.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public record ChatClientRequest(Prompt prompt, Map<String, Object> context) {
3737
Assert.notNull(prompt, "prompt cannot be null");
3838
Assert.notNull(context, "context cannot be null");
3939
Assert.noNullElements(context.keySet(), "context keys cannot be null");
40+
Assert.noNullElements(context.values(), "context values cannot be null");
4041
}
4142

4243
public ChatClientRequest copy() {
@@ -68,12 +69,15 @@ public Builder prompt(Prompt prompt) {
6869

6970
public Builder context(Map<String, Object> context) {
7071
Assert.notNull(context, "context cannot be null");
72+
Assert.noNullElements(context.keySet(), "context keys cannot be null");
73+
Assert.noNullElements(context.values(), "context values cannot be null");
7174
this.context.putAll(context);
7275
return this;
7376
}
7477

7578
public Builder context(String key, Object value) {
7679
Assert.notNull(key, "key cannot be null");
80+
Assert.notNull(value, "value cannot be null");
7781
this.context.put(key, value);
7882
return this;
7983
}

spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/ChatClientResponse.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public record ChatClientResponse(@Nullable ChatResponse chatResponse, Map<String
3636
public ChatClientResponse {
3737
Assert.notNull(context, "context cannot be null");
3838
Assert.noNullElements(context.keySet(), "context keys cannot be null");
39+
Assert.noNullElements(context.values(), "context values cannot be null");
3940
}
4041

4142
public ChatClientResponse copy() {
@@ -66,12 +67,15 @@ public Builder chatResponse(@Nullable ChatResponse chatResponse) {
6667

6768
public Builder context(Map<String, Object> context) {
6869
Assert.notNull(context, "context cannot be null");
70+
Assert.noNullElements(context.keySet(), "context keys cannot be null");
71+
Assert.noNullElements(context.values(), "context values cannot be null");
6972
this.context.putAll(context);
7073
return this;
7174
}
7275

7376
public Builder context(String key, Object value) {
7477
Assert.notNull(key, "key cannot be null");
78+
Assert.notNull(value, "value cannot be null");
7579
this.context.put(key, value);
7680
return this;
7781
}

spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/ChatClientRequestTests.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,30 @@ void whenContextHasNullKeysThenThrow() {
6262
.hasMessage("context keys cannot be null");
6363
}
6464

65+
@Test
66+
void whenContextHasNullValuesThenThrow() {
67+
Map<String, Object> context = new HashMap<>();
68+
context.put("key", null);
69+
70+
assertThatThrownBy(() -> new ChatClientRequest(new Prompt(), context))
71+
.isInstanceOf(IllegalArgumentException.class)
72+
.hasMessage("context values cannot be null");
73+
74+
assertThatThrownBy(() -> ChatClientRequest.builder().prompt(new Prompt()).context(context).build())
75+
.isInstanceOf(IllegalArgumentException.class)
76+
.hasMessage("context values cannot be null");
77+
}
78+
79+
@Test
80+
void whenBuilderContextMapHasNullKeyThenThrow() {
81+
Map<String, Object> context = new HashMap<>();
82+
context.put(null, "value");
83+
84+
assertThatThrownBy(() -> ChatClientRequest.builder().prompt(new Prompt()).context(context).build())
85+
.isInstanceOf(IllegalArgumentException.class)
86+
.hasMessage("context keys cannot be null");
87+
}
88+
6589
@Test
6690
void whenCopyThenImmutableContext() {
6791
Map<String, Object> context = new HashMap<>();
@@ -86,6 +110,13 @@ void whenMutateThenImmutableContext() {
86110
assertThat(copy.context()).isEqualTo(Map.of("key", "newValue"));
87111
}
88112

113+
@Test
114+
void whenBuilderAddsNullValueThenThrow() {
115+
assertThatThrownBy(() -> ChatClientRequest.builder().prompt(new Prompt()).context("key", null).build())
116+
.isInstanceOf(IllegalArgumentException.class)
117+
.hasMessage("value cannot be null");
118+
}
119+
89120
@Test
90121
void whenBuilderWithMultipleContextEntriesThenSuccess() {
91122
Prompt prompt = new Prompt("test message");

spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/ChatClientResponseTests.java

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,19 @@ void whenContextHasNullKeysThenThrow() {
5353
.hasMessage("context keys cannot be null");
5454
}
5555

56+
@Test
57+
void whenContextHasNullValuesThenThrow() {
58+
Map<String, Object> context = new HashMap<>();
59+
context.put("key", null);
60+
61+
assertThatThrownBy(() -> new ChatClientResponse(null, context)).isInstanceOf(IllegalArgumentException.class)
62+
.hasMessage("context values cannot be null");
63+
64+
assertThatThrownBy(() -> ChatClientResponse.builder().chatResponse(null).context(context).build())
65+
.isInstanceOf(IllegalArgumentException.class)
66+
.hasMessage("context values cannot be null");
67+
}
68+
5669
@Test
5770
void whenCopyThenImmutableContext() {
5871
Map<String, Object> context = new HashMap<>();
@@ -120,16 +133,15 @@ void whenEmptyContextThenCreateSuccessfully() {
120133
}
121134

122135
@Test
123-
void whenContextWithNullValuesThenCreateSuccessfully() {
136+
void whenContextWithNullValuesThenThrow() {
124137
ChatResponse chatResponse = mock(ChatResponse.class);
125138
Map<String, Object> context = new HashMap<>();
126139
context.put("key1", "value1");
127140
context.put("key2", null);
128141

129-
ChatClientResponse response = new ChatClientResponse(chatResponse, context);
130-
131-
assertThat(response.context()).containsEntry("key1", "value1");
132-
assertThat(response.context()).containsEntry("key2", null);
142+
assertThatThrownBy(() -> new ChatClientResponse(chatResponse, context))
143+
.isInstanceOf(IllegalArgumentException.class)
144+
.hasMessage("context values cannot be null");
133145
}
134146

135147
@Test
@@ -166,6 +178,23 @@ void whenBuilderWithoutChatResponseThenCreateWithNull() {
166178
assertThat(response.chatResponse()).isNull();
167179
}
168180

181+
@Test
182+
void whenBuilderContextMapHasNullKeyThenThrow() {
183+
Map<String, Object> context = new HashMap<>();
184+
context.put(null, "value");
185+
186+
assertThatThrownBy(() -> ChatClientResponse.builder().context(context).build())
187+
.isInstanceOf(IllegalArgumentException.class)
188+
.hasMessage("context keys cannot be null");
189+
}
190+
191+
@Test
192+
void whenBuilderAddsNullValueThenThrow() {
193+
assertThatThrownBy(() -> ChatClientResponse.builder().context("key", null).build())
194+
.isInstanceOf(IllegalArgumentException.class)
195+
.hasMessage("value cannot be null");
196+
}
197+
169198
@Test
170199
void whenComplexObjectsInContextThenPreserveCorrectly() {
171200
ChatResponse chatResponse = mock(ChatResponse.class);

0 commit comments

Comments
 (0)