jungm commented on code in PR #1328:
URL: https://github.com/apache/tomee/pull/1328#discussion_r1693951730


##########
tomee/tomee-security/src/main/java/org/apache/tomee/security/http/SavedRequest.java:
##########
@@ -102,12 +102,20 @@ public Enumeration<String> getHeaderNames() {
 
             @Override
             public Enumeration<String> getHeaders(String name) {
-                return Collections.enumeration(headers.get(name));
+                List<String> header = headers.get(name);
+                if (header == null) {
+                    header = Collections.emptyList();
+                }
+                return Collections.enumeration(header);
             }
 
             @Override
             public String getHeader(String name) {
-                return headers.get(name).get(0);
+                List<String> header = headers.get(name);
+                if (header == null || header.isEmpty()) {
+                    return null;

Review Comment:
   Servlet 6.0 spec says:
   > Returns the value of the specified request header as a String. If the 
request did not include a header of the specified name, this method returns 
null. If there are multiple headers with the same name, this method returns the 
first head in the request. The header name is case insensitive. You can use 
this method with any request header.
   
   IMO this means we must not alter the header value if it is present. From a 
quick glance this is also how tomcat/jetty work



##########
tomee/tomee-security/src/main/java/org/apache/tomee/security/http/SavedRequest.java:
##########
@@ -102,12 +102,20 @@ public Enumeration<String> getHeaderNames() {
 
             @Override
             public Enumeration<String> getHeaders(String name) {
-                return Collections.enumeration(headers.get(name));
+                List<String> header = headers.get(name);
+                if (header == null) {
+                    header = Collections.emptyList();
+                }
+                return Collections.enumeration(header);
             }
 
             @Override
             public String getHeader(String name) {
-                return headers.get(name).get(0);
+                List<String> header = headers.get(name);

Review Comment:
   in `getHeaders(String)` you assume `headers`may be null, this check is 
missing here



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@tomee.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to