Spring Framework server Web Observations DoS Vulnerability

@@ -108,7 +108,7 @@ By default, the following `KeyValues` are created:
 |===
 |Name | Description
 |`exception` _(required)_|Name of the exception thrown during the exchange, or `KeyValue#NONE_VALUE`} if no exception happened.
-|`method` _(required)_|Name of HTTP request method or `"none"` if the request was not received properly.
+|`method` _(required)_|Name of HTTP request method or `"none"` if not a well-known method.
 |`outcome` _(required)_|Outcome of the HTTP server exchange.
 |`status` _(required)_|HTTP response raw status code, or `"UNKNOWN"` if no response was created.
 |`uri` _(required)_|URI pattern for the matching handler if available, falling back to `REDIRECTION` for 3xx responses, `NOT_FOUND` for 404 responses, `root` for requests with no path info, and `UNKNOWN` for all other requests.
@@ -141,7 +141,7 @@ By default, the following `KeyValues` are created:
 |===
 |Name | Description
 |`exception` _(required)_|Name of the exception thrown during the exchange, or `"none"` if no exception happened.
-|`method` _(required)_|Name of HTTP request method or `"none"` if the request was not received properly.
+|`method` _(required)_|Name of HTTP request method or `"none"` if not a well-known method.
 |`outcome` _(required)_|Outcome of the HTTP server exchange.
 |`status` _(required)_|HTTP response raw status code, or `"UNKNOWN"` if no response was created.
 |`uri` _(required)_|URI pattern for the matching handler if available, falling back to `REDIRECTION` for 3xx responses, `NOT_FOUND` for 404 responses, `root` for requests with no path info, and `UNKNOWN` for all other requests.
@@ -174,7 +174,7 @@ Instrumentation uses the `org.springframework.http.client.observation.ClientRequ
 [cols="a,a"]
 |===
 |Name | Description
-|`method` _(required)_|Name of HTTP request method or `"none"` if the request could not be created.
+|`method` _(required)_|Name of HTTP request method or `"none"` if not a well-known method.
 |`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. Only the path part of the URI is considered.
 |`client.name` _(required)_|Client name derived from the request URI host.
 |`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received.
@@ -203,7 +203,7 @@ Instrumentation uses the `org.springframework.web.reactive.function.client.Clien
 [cols="a,a"]
 |===
 |Name | Description
-|`method` _(required)_|Name of HTTP request method or `"none"` if the request could not be created.
+|`method` _(required)_|Name of HTTP request method or `"none"` if not a well-known method.
 |`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. Only the path part of the URI is considered.
 |`client.name` _(required)_|Client name derived from the request URI host.
 |`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received.

@@ -16,9 +16,14 @@
 
 package org.springframework.http.server.observation;
 
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
 import io.micrometer.common.KeyValue;
 import io.micrometer.common.KeyValues;
 
+import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.HttpStatusCode;
 import org.springframework.http.server.observation.ServerHttpObservationDocumentation.HighCardinalityKeyNames;
@@ -55,6 +60,8 @@ public class DefaultServerRequestObservationConvention implements ServerRequestO
 
 	private static final KeyValue HTTP_URL_UNKNOWN = KeyValue.of(HighCardinalityKeyNames.HTTP_URL, "UNKNOWN");
 
+	private static final Set<String> HTTP_METHODS = Stream.of(HttpMethod.values()).map(HttpMethod::name).collect(Collectors.toUnmodifiableSet());
+
 
 	private final String name;
 
@@ -102,9 +109,13 @@ public KeyValues getHighCardinalityKeyValues(ServerRequestObservationContext con
 	}
 
 	protected KeyValue method(ServerRequestObservationContext context) {
-		return (context.getCarrier() != null) ?
-				KeyValue.of(LowCardinalityKeyNames.METHOD, context.getCarrier().getMethod()) :
-				METHOD_UNKNOWN;
+		if (context.getCarrier() != null) {
+			String httpMethod = context.getCarrier().getMethod();
+			if (HTTP_METHODS.contains(httpMethod)) {
+				return KeyValue.of(LowCardinalityKeyNames.METHOD, httpMethod);
+			}
+		}
+		return METHOD_UNKNOWN;
 	}
 
 	protected KeyValue status(ServerRequestObservationContext context) {

@@ -16,9 +16,12 @@
 
 package org.springframework.http.server.reactive.observation;
 
+import java.util.Set;
+
 import io.micrometer.common.KeyValue;
 import io.micrometer.common.KeyValues;
 
+import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.HttpStatusCode;
 import org.springframework.http.server.reactive.observation.ServerHttpObservationDocumentation.HighCardinalityKeyNames;
@@ -55,6 +58,8 @@ public class DefaultServerRequestObservationConvention implements ServerRequestO
 
 	private static final KeyValue HTTP_URL_UNKNOWN = KeyValue.of(HighCardinalityKeyNames.HTTP_URL, "UNKNOWN");
 
+	private static final Set<HttpMethod> HTTP_METHODS = Set.of(HttpMethod.values());
+
 
 	private final String name;
 
@@ -102,9 +107,13 @@ public KeyValues getHighCardinalityKeyValues(ServerRequestObservationContext con
 	}
 
 	protected KeyValue method(ServerRequestObservationContext context) {
-		return (context.getCarrier() != null) ?
-				KeyValue.of(LowCardinalityKeyNames.METHOD, context.getCarrier().getMethod().name()) :
-				METHOD_UNKNOWN;
+		if (context.getCarrier() != null) {
+			HttpMethod method = context.getCarrier().getMethod();
+			if (HTTP_METHODS.contains(method)) {
+				return KeyValue.of(LowCardinalityKeyNames.METHOD, method.name());
+			}
+		}
+		return METHOD_UNKNOWN;
 	}
 
 	protected KeyValue status(ServerRequestObservationContext context) {

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -124,4 +124,17 @@ void addsKeyValuesForNotFoundExchange() {
 				.contains(KeyValue.of("http.url", "/test/notFound"));
 	}
 
+	@Test
+	void addsKeyValuesForUnknownHttpMethodExchange() {
+		this.request.setMethod("SPRING");
+		this.request.setRequestURI("/test");
+		this.response.setStatus(404);
+
+		assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(5)
+				.contains(KeyValue.of("method", "UNKNOWN"), KeyValue.of("uri", "NOT_FOUND"), KeyValue.of("status", "404"),
+						KeyValue.of("exception", "none"), KeyValue.of("outcome", "CLIENT_ERROR"));
+		assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(1)
+				.contains(KeyValue.of("http.url", "/test"));
+	}
+
 }

@@ -20,6 +20,7 @@
 import io.micrometer.observation.Observation;
 import org.junit.jupiter.api.Test;
 
+import org.springframework.http.HttpMethod;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest;
 import org.springframework.web.testfixture.server.MockServerWebExchange;
@@ -172,4 +173,17 @@ void supportsNullStatusCode() {
 						KeyValue.of("exception", "none"), KeyValue.of("outcome", "UNKNOWN"));
 	}
 
+	@Test
+	void addsKeyValuesForUnknownHttpMethodExchange() {
+		ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.method(HttpMethod.valueOf("SPRING"), "/test"));
+		ServerRequestObservationContext context = new ServerRequestObservationContext(exchange.getRequest(), exchange.getResponse(), exchange.getAttributes());
+		exchange.getResponse().setRawStatusCode(404);
+
+		assertThat(this.convention.getLowCardinalityKeyValues(context)).hasSize(5)
+				.contains(KeyValue.of("method", "UNKNOWN"), KeyValue.of("uri", "NOT_FOUND"), KeyValue.of("status", "404"),
+						KeyValue.of("exception", "none"), KeyValue.of("outcome", "CLIENT_ERROR"));
+		assertThat(this.convention.getHighCardinalityKeyValues(context)).hasSize(1)
+				.contains(KeyValue.of("http.url", "/test"));
+	}
+
 }