Copilot commented on code in PR #2472:
URL: https://github.com/apache/tomee/pull/2472#discussion_r2894115555
##########
tomee/tomee-security/src/main/java/org/apache/tomee/security/http/SavedRequest.java:
##########
@@ -184,6 +203,14 @@ public void setQueryString(String queryString) {
this.queryString = queryString;
}
+ public Map<String, String[]> getParameterMap() {
+ return parameterMap;
+ }
+
+ public void setParameterMap(Map<String, String[]> parameterMap) {
+ this.parameterMap = parameterMap;
Review Comment:
`SavedRequest.getParameterMap()` can return null (e.g., when deserializing
JSON produced by older versions without the `parameterMap` field), but the
updated tests and the `mask()` wrapper logic assume a safe default. Consider
returning an empty map when `parameterMap` is null and/or initializing
`parameterMap` to `Collections.emptyMap()` during `fromJson()`/deserialization
to avoid NPEs and ensure backward compatibility.
```suggestion
return parameterMap != null ? parameterMap : Collections.emptyMap();
}
public void setParameterMap(Map<String, String[]> parameterMap) {
this.parameterMap = parameterMap != null ? parameterMap :
Collections.emptyMap();
```
##########
tomee/tomee-security/src/main/java/org/apache/tomee/security/http/SavedRequest.java:
##########
@@ -56,25 +56,23 @@ public class SavedRequest implements Serializable {
private String method;
private String url;
private String queryString;
+ private Map<String, String[]> parameterMap;
public static SavedRequest fromRequest(HttpServletRequest request) {
- Cookie[] cookies = request.getCookies();
Map<String, List<String>> headers = new HashMap<>();
Enumeration<String> headerNames = request.getHeaderNames();
while (headerNames.hasMoreElements()) {
String name = headerNames.nextElement();
headers.put(name, Collections.list(request.getHeaders(name)));
}
- String method = request.getMethod();
- String queryString = request.getQueryString();
-
SavedRequest result = new SavedRequest();
- result.setCookies(cookies);
+ result.setCookies(request.getCookies());
result.setHeaders(headers);
- result.setMethod(method);
+ result.setMethod(request.getMethod());
result.setUrl(request.getRequestURL().toString());
- result.setQueryString(queryString);
+ result.setQueryString(request.getQueryString());
+ result.setParameterMap(request.getParameterMap());
Review Comment:
`fromRequest()` stores `request.getParameterMap()` directly into the
session-serializable `SavedRequest`. The Servlet API doesn’t guarantee the
returned `Map` implementation is `Serializable` (and it can be a live,
container-specific view), which can cause session passivation/replication
failures or unexpected mutation. Prefer making a defensive, `Serializable` copy
(and ideally cloning the `String[]` values) before storing it.
```suggestion
// Make a defensive, serializable copy of the parameter map
Map<String, String[]> originalParameterMap =
request.getParameterMap();
Map<String, String[]> copiedParameterMap = new
HashMap<>(originalParameterMap.size());
for (Map.Entry<String, String[]> entry :
originalParameterMap.entrySet()) {
String key = entry.getKey();
String[] value = entry.getValue();
copiedParameterMap.put(key, value != null ? value.clone() :
null);
}
result.setParameterMap(copiedParameterMap);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
