[
https://issues.apache.org/jira/browse/TOMEE-4584?focusedWorklogId=1008175&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1008175
]
ASF GitHub Bot logged work on TOMEE-4584:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 06/Mar/26 06:39
Start Date: 06/Mar/26 06:39
Worklog Time Spent: 10m
Work Description: 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);
```
Issue Time Tracking
-------------------
Worklog Id: (was: 1008175)
Time Spent: 0.5h (was: 20m)
> OpenIdAuthenticationMechanism redirectToOriginalResource does not restore
> parameterMap
> --------------------------------------------------------------------------------------
>
> Key: TOMEE-4584
> URL: https://issues.apache.org/jira/browse/TOMEE-4584
> Project: TomEE
> Issue Type: Bug
> Affects Versions: 10.1.4
> Reporter: Markus Jung
> Assignee: Markus Jung
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> See spec/javadoc of OpenIdAuthenticationMechanismDefinition:
>
> {code:java}
> /**
> * Optional. Automatically redirects the caller (the end-user) from
> * the redirect URI defined by the <code>redirectURI</code> attribute
> * to the resource the end-user originally requested in a "login to continue"
> * scenario.
> *
> * <p>
> * After arriving at the original requested resource, the runtime restores
> * the request as it originally happened, including cookies, headers, the
> * request method and the request parameters in the same way as done when
> * using the {@link LoginToContinue} feature.
> *
> * @return
> */
> boolean redirectToOriginalResource() default false; {code}
>
> The following methods do not work as intended with redirectToOriginalResource:
> * getParameterNames()
> * getParameterValues(String name)
> * getParameter(String name)
> * getParameterMap()
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
