Skip to content

Commit 2561471

Browse files
mhalbrittermhalbritter
committed
Fail if superfluous properties are used in property metadata
Closes gh-37597
1 parent 970c226 commit 2561471

File tree

2 files changed

+233
-25
lines changed
  • spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src

2 files changed

+233
-25
lines changed

‎spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshaller.java

+72-23
Original file line numberDiff line numberDiff line change
@@ -18,30 +18,31 @@
1818

1919
import java.io.IOException;
2020
import java.io.InputStream;
21-
import java.io.InputStreamReader;
2221
import java.io.OutputStream;
2322
import java.nio.charset.StandardCharsets;
2423
import java.util.ArrayList;
24+
import java.util.Arrays;
2525
import java.util.HashMap;
2626
import java.util.Iterator;
2727
import java.util.List;
2828
import java.util.Map;
29+
import java.util.Set;
30+
import java.util.TreeSet;
2931

3032
import org.springframework.boot.configurationprocessor.json.JSONArray;
3133
import org.springframework.boot.configurationprocessor.json.JSONObject;
3234
import org.springframework.boot.configurationprocessor.metadata.ItemMetadata.ItemType;
3335

3436
/**
35-
* Marshaller to write {@link ConfigurationMetadata} as JSON.
37+
* Marshaller to read and write {@link ConfigurationMetadata} as JSON.
3638
*
3739
* @author Stephane Nicoll
3840
* @author Phillip Webb
41+
* @author Moritz Halbritter
3942
* @since 1.2.0
4043
*/
4144
public class JsonMarshaller {
4245

43-
private static final int BUFFER_SIZE = 4098;
44-
4546
public void write(ConfigurationMetadata metadata, OutputStream outputStream) throws IOException {
4647
try {
4748
JSONObject object = new JSONObject();
@@ -65,42 +66,53 @@ public void write(ConfigurationMetadata metadata, OutputStream outputStream) thr
6566
public ConfigurationMetadata read(InputStream inputStream) throws Exception {
6667
ConfigurationMetadata metadata = new ConfigurationMetadata();
6768
JSONObject object = new JSONObject(toString(inputStream));
69+
JsonPath path = JsonPath.root();
70+
checkAllowedKeys(object, path, "groups", "properties", "hints");
6871
JSONArray groups = object.optJSONArray("groups");
6972
if (groups != null) {
7073
for (int i = 0; i < groups.length(); i++) {
71-
metadata.add(toItemMetadata((JSONObject) groups.get(i), ItemType.GROUP));
74+
metadata
75+
.add(toItemMetadata((JSONObject) groups.get(i), path.resolve("groups").index(i), ItemType.GROUP));
7276
}
7377
}
7478
JSONArray properties = object.optJSONArray("properties");
7579
if (properties != null) {
7680
for (int i = 0; i < properties.length(); i++) {
77-
metadata.add(toItemMetadata((JSONObject) properties.get(i), ItemType.PROPERTY));
81+
metadata.add(toItemMetadata((JSONObject) properties.get(i), path.resolve("properties").index(i),
82+
ItemType.PROPERTY));
7883
}
7984
}
8085
JSONArray hints = object.optJSONArray("hints");
8186
if (hints != null) {
8287
for (int i = 0; i < hints.length(); i++) {
83-
metadata.add(toItemHint((JSONObject) hints.get(i)));
88+
metadata.add(toItemHint((JSONObject) hints.get(i), path.resolve("hints").index(i)));
8489
}
8590
}
8691
return metadata;
8792
}
8893

89-
private ItemMetadata toItemMetadata(JSONObject object, ItemType itemType) throws Exception {
94+
private ItemMetadata toItemMetadata(JSONObject object, JsonPath path, ItemType itemType) throws Exception {
95+
switch (itemType) {
96+
case GROUP -> checkAllowedKeys(object, path, "name", "type", "description", "sourceType", "sourceMethod");
97+
case PROPERTY -> checkAllowedKeys(object, path, "name", "type", "description", "sourceType", "defaultValue",
98+
"deprecation", "deprecated");
99+
}
90100
String name = object.getString("name");
91101
String type = object.optString("type", null);
92102
String description = object.optString("description", null);
93103
String sourceType = object.optString("sourceType", null);
94104
String sourceMethod = object.optString("sourceMethod", null);
95105
Object defaultValue = readItemValue(object.opt("defaultValue"));
96-
ItemDeprecation deprecation = toItemDeprecation(object);
106+
ItemDeprecation deprecation = toItemDeprecation(object, path);
97107
return new ItemMetadata(itemType, name, null, type, sourceType, sourceMethod, description, defaultValue,
98108
deprecation);
99109
}
100110

101-
private ItemDeprecation toItemDeprecation(JSONObject object) throws Exception {
111+
private ItemDeprecation toItemDeprecation(JSONObject object, JsonPath path) throws Exception {
102112
if (object.has("deprecation")) {
103113
JSONObject deprecationJsonObject = object.getJSONObject("deprecation");
114+
checkAllowedKeys(deprecationJsonObject, path.resolve("deprecation"), "level", "reason", "replacement",
115+
"since");
104116
ItemDeprecation deprecation = new ItemDeprecation();
105117
deprecation.setLevel(deprecationJsonObject.optString("level", null));
106118
deprecation.setReason(deprecationJsonObject.optString("reason", null));
@@ -111,32 +123,35 @@ private ItemDeprecation toItemDeprecation(JSONObject object) throws Exception {
111123
return object.optBoolean("deprecated") ? new ItemDeprecation() : null;
112124
}
113125

114-
private ItemHint toItemHint(JSONObject object) throws Exception {
126+
private ItemHint toItemHint(JSONObject object, JsonPath path) throws Exception {
127+
checkAllowedKeys(object, path, "name", "values", "providers");
115128
String name = object.getString("name");
116129
List<ItemHint.ValueHint> values = new ArrayList<>();
117130
if (object.has("values")) {
118131
JSONArray valuesArray = object.getJSONArray("values");
119132
for (int i = 0; i < valuesArray.length(); i++) {
120-
values.add(toValueHint((JSONObject) valuesArray.get(i)));
133+
values.add(toValueHint((JSONObject) valuesArray.get(i), path.resolve("values").index(i)));
121134
}
122135
}
123136
List<ItemHint.ValueProvider> providers = new ArrayList<>();
124137
if (object.has("providers")) {
125138
JSONArray providersObject = object.getJSONArray("providers");
126139
for (int i = 0; i < providersObject.length(); i++) {
127-
providers.add(toValueProvider((JSONObject) providersObject.get(i)));
140+
providers.add(toValueProvider((JSONObject) providersObject.get(i), path.resolve("providers").index(i)));
128141
}
129142
}
130143
return new ItemHint(name, values, providers);
131144
}
132145

133-
private ItemHint.ValueHint toValueHint(JSONObject object) throws Exception {
146+
private ItemHint.ValueHint toValueHint(JSONObject object, JsonPath path) throws Exception {
147+
checkAllowedKeys(object, path, "value", "description");
134148
Object value = readItemValue(object.get("value"));
135149
String description = object.optString("description", null);
136150
return new ItemHint.ValueHint(value, description);
137151
}
138152

139-
private ItemHint.ValueProvider toValueProvider(JSONObject object) throws Exception {
153+
private ItemHint.ValueProvider toValueProvider(JSONObject object, JsonPath path) throws Exception {
154+
checkAllowedKeys(object, path, "name", "parameters");
140155
String name = object.getString("name");
141156
Map<String, Object> parameters = new HashMap<>();
142157
if (object.has("parameters")) {
@@ -162,14 +177,48 @@ private Object readItemValue(Object value) throws Exception {
162177
}
163178

164179
private String toString(InputStream inputStream) throws IOException {
165-
StringBuilder out = new StringBuilder();
166-
InputStreamReader reader = new InputStreamReader(inputStream, StandardCharsets.UTF_8);
167-
char[] buffer = new char[BUFFER_SIZE];
168-
int bytesRead;
169-
while ((bytesRead = reader.read(buffer)) != -1) {
170-
out.append(buffer, 0, bytesRead);
171-
}
172-
return out.toString();
180+
return new String(inputStream.readAllBytes(), StandardCharsets.UTF_8);
181+
}
182+
183+
@SuppressWarnings("unchecked")
184+
private void checkAllowedKeys(JSONObject object, JsonPath path, String... allowedKeys) {
185+
Set<String> availableKeys = new TreeSet<>();
186+
object.keys().forEachRemaining((key) -> availableKeys.add((String) key));
187+
Arrays.stream(allowedKeys).forEach(availableKeys::remove);
188+
if (!availableKeys.isEmpty()) {
189+
throw new IllegalStateException("Expected only keys %s, but found additional keys %s. Path: %s"
190+
.formatted(new TreeSet<>(Arrays.asList(allowedKeys)), availableKeys, path));
191+
}
192+
}
193+
194+
private static final class JsonPath {
195+
196+
private final String path;
197+
198+
private JsonPath(String path) {
199+
this.path = path;
200+
}
201+
202+
JsonPath resolve(String path) {
203+
if (this.path.endsWith(".")) {
204+
return new JsonPath(this.path + path);
205+
}
206+
return new JsonPath(this.path + "." + path);
207+
}
208+
209+
JsonPath index(int index) {
210+
return resolve("[%d]".formatted(index));
211+
}
212+
213+
@Override
214+
public String toString() {
215+
return this.path;
216+
}
217+
218+
static JsonPath root() {
219+
return new JsonPath(".");
220+
}
221+
173222
}
174223

175224
}

‎spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java

+161-2
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@
2020
import java.io.ByteArrayOutputStream;
2121
import java.io.IOException;
2222
import java.io.InputStream;
23+
import java.nio.charset.StandardCharsets;
2324
import java.util.Arrays;
2425
import java.util.Collections;
2526

2627
import org.junit.jupiter.api.Test;
2728

2829
import static org.assertj.core.api.Assertions.assertThat;
30+
import static org.assertj.core.api.Assertions.assertThatException;
2931

3032
/**
3133
* Tests for {@link JsonMarshaller}.
@@ -38,14 +40,15 @@ class JsonMarshallerTests {
3840
@Test
3941
void marshallAndUnmarshal() throws Exception {
4042
ConfigurationMetadata metadata = new ConfigurationMetadata();
41-
metadata.add(ItemMetadata.newProperty("a", "b", StringBuffer.class.getName(), InputStream.class.getName(),
42-
"sourceMethod", "desc", "x", new ItemDeprecation("Deprecation comment", "b.c.d", "1.2.3")));
43+
metadata.add(ItemMetadata.newProperty("a", "b", StringBuffer.class.getName(), InputStream.class.getName(), null,
44+
"desc", "x", new ItemDeprecation("Deprecation comment", "b.c.d", "1.2.3")));
4345
metadata.add(ItemMetadata.newProperty("b.c.d", null, null, null, null, null, null, null));
4446
metadata.add(ItemMetadata.newProperty("c", null, null, null, null, null, 123, null));
4547
metadata.add(ItemMetadata.newProperty("d", null, null, null, null, null, true, null));
4648
metadata.add(ItemMetadata.newProperty("e", null, null, null, null, null, new String[] { "y", "n" }, null));
4749
metadata.add(ItemMetadata.newProperty("f", null, null, null, null, null, new Boolean[] { true, false }, null));
4850
metadata.add(ItemMetadata.newGroup("d", null, null, null));
51+
metadata.add(ItemMetadata.newGroup("e", null, null, "sourceMethod"));
4952
metadata.add(ItemHint.newHint("a.b"));
5053
metadata.add(ItemHint.newHint("c", new ItemHint.ValueHint(123, "hey"), new ItemHint.ValueHint(456, null)));
5154
metadata.add(new ItemHint("d", null,
@@ -66,6 +69,7 @@ void marshallAndUnmarshal() throws Exception {
6669
assertThat(read).has(Metadata.withProperty("e").withDefaultValue(new String[] { "y", "n" }));
6770
assertThat(read).has(Metadata.withProperty("f").withDefaultValue(new Object[] { true, false }));
6871
assertThat(read).has(Metadata.withGroup("d"));
72+
assertThat(read).has(Metadata.withGroup("e").fromSourceMethod("sourceMethod"));
6973
assertThat(read).has(Metadata.withHint("a.b"));
7074
assertThat(read).has(Metadata.withHint("c").withValue(0, 123, "hey").withValue(1, 456, null));
7175
assertThat(read).has(Metadata.withHint("d").withProvider("first", "target", "foo").withProvider("second"));
@@ -170,4 +174,159 @@ void orderingForSamePropertyNamesWithNullSourceType() throws IOException {
170174
"\"java.lang.Boolean\"", "\"com.example.bravo.aaa\"", "\"java.lang.Integer\"", "\"com.example.Bar");
171175
}
172176

177+
@Test
178+
void shouldCheckRootFields() {
179+
String json = """
180+
{
181+
"groups": [], "properties": [], "hints": [], "dummy": []
182+
}""";
183+
assertThatException().isThrownBy(() -> read(json))
184+
.withMessage("Expected only keys [groups, hints, properties], but found additional keys [dummy]. Path: .");
185+
}
186+
187+
@Test
188+
void shouldCheckGroupFields() {
189+
String json = """
190+
{
191+
"groups": [
192+
{
193+
"name": "g",
194+
"type": "java.lang.String",
195+
"description": "Some description",
196+
"sourceType": "java.lang.String",
197+
"sourceMethod": "some()",
198+
"dummy": "dummy"
199+
}
200+
], "properties": [], "hints": []
201+
}""";
202+
assertThatException().isThrownBy(() -> read(json))
203+
.withMessage(
204+
"Expected only keys [description, name, sourceMethod, sourceType, type], but found additional keys [dummy]. Path: .groups.[0]");
205+
}
206+
207+
@Test
208+
void shouldCheckPropertyFields() {
209+
String json = """
210+
{
211+
"groups": [], "properties": [
212+
{
213+
"name": "name",
214+
"type": "java.lang.String",
215+
"description": "Some description",
216+
"sourceType": "java.lang.String",
217+
"defaultValue": "value",
218+
"deprecation": {
219+
"level": "warning",
220+
"reason": "some reason",
221+
"replacement": "name-new",
222+
"since": "v17"
223+
},
224+
"deprecated": true,
225+
"dummy": "dummy"
226+
}
227+
], "hints": []
228+
}""";
229+
assertThatException().isThrownBy(() -> read(json))
230+
.withMessage(
231+
"Expected only keys [defaultValue, deprecated, deprecation, description, name, sourceType, type], but found additional keys [dummy]. Path: .properties.[0]");
232+
}
233+
234+
@Test
235+
void shouldCheckPropertyDeprecationFields() {
236+
String json = """
237+
{
238+
"groups": [], "properties": [
239+
{
240+
"name": "name",
241+
"type": "java.lang.String",
242+
"description": "Some description",
243+
"sourceType": "java.lang.String",
244+
"defaultValue": "value",
245+
"deprecation": {
246+
"level": "warning",
247+
"reason": "some reason",
248+
"replacement": "name-new",
249+
"since": "v17",
250+
"dummy": "dummy"
251+
},
252+
"deprecated": true
253+
}
254+
], "hints": []
255+
}""";
256+
assertThatException().isThrownBy(() -> read(json))
257+
.withMessage(
258+
"Expected only keys [level, reason, replacement, since], but found additional keys [dummy]. Path: .properties.[0].deprecation");
259+
}
260+
261+
@Test
262+
void shouldCheckHintFields() {
263+
String json = """
264+
{
265+
"groups": [], "properties": [], "hints": [
266+
{
267+
"name": "name",
268+
"values": [],
269+
"providers": [],
270+
"dummy": "dummy"
271+
}
272+
]
273+
}""";
274+
assertThatException().isThrownBy(() -> read(json))
275+
.withMessage(
276+
"Expected only keys [name, providers, values], but found additional keys [dummy]. Path: .hints.[0]");
277+
}
278+
279+
@Test
280+
void shouldCheckHintValueFields() {
281+
String json = """
282+
{
283+
"groups": [], "properties": [], "hints": [
284+
{
285+
"name": "name",
286+
"values": [
287+
{
288+
"value": "value",
289+
"description": "some description",
290+
"dummy": "dummy"
291+
}
292+
],
293+
"providers": []
294+
}
295+
]
296+
}""";
297+
assertThatException().isThrownBy(() -> read(json))
298+
.withMessage(
299+
"Expected only keys [description, value], but found additional keys [dummy]. Path: .hints.[0].values.[0]");
300+
}
301+
302+
@Test
303+
void shouldCheckHintProviderFields() {
304+
String json = """
305+
{
306+
"groups": [], "properties": [], "hints": [
307+
{
308+
"name": "name",
309+
"values": [],
310+
"providers": [
311+
{
312+
"name": "name",
313+
"parameters": {
314+
"target": "jakarta.servlet.http.HttpServlet"
315+
},
316+
"dummy": "dummy"
317+
}
318+
]
319+
}
320+
]
321+
}""";
322+
assertThatException().isThrownBy(() -> read(json))
323+
.withMessage(
324+
"Expected only keys [name, parameters], but found additional keys [dummy]. Path: .hints.[0].providers.[0]");
325+
}
326+
327+
private void read(String json) throws Exception {
328+
JsonMarshaller marshaller = new JsonMarshaller();
329+
marshaller.read(new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)));
330+
}
331+
173332
}

0 commit comments

Comments
 (0)