Le 09/11/2019 à 15:17, Mathieu Lirzin a écrit :
Hello Jacques,
jler...@apache.org writes:
+++
b/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java
@@ -96,6 +99,19 @@ public class SeoContextFilter implements Filter {
HttpServletResponse httpResponse = (HttpServletResponse) response;
String uri = httpRequest.getRequestURI();
+
+ Map<String, String[]> parameterMap =request.getParameterMap();
+ if (parameterMap != null) {
The documentation of ‘getParameterMap’ is not stating that the return
value can be ‘null’ contrary to what is done in other methods of the
same class so I guess we could assume that an empty map is returned.
Good point, I'll amend that
+ List<BasicNameValuePair> params = new
ArrayList<BasicNameValuePair>();
^^^
Unnecessary type
declaration
+ request.getParameterMap().forEach((name, values) -> {
+ for(String value : values) {
+ params.add(new BasicNameValuePair(name, value));
+ }
+ });
+ String queryString = URLEncodedUtils.format(params,
Charset.forName("UTF-8"));
+ uri = uri + "?" + queryString;
+ }
Is there any reason why you are using ‘URLEncodedUtils.format’ instead
of simply retrieving the query string from the servlet request, like
this?
--8<---------------cut here---------------start------------->8---
String uri = httpRequest.getRequestURI();
String queryString = httpRequest.getQueryString();
if (queryString != null) {
uri = uri + "?" + queryString;
}
boolean forwarded = forwardUri(httpResponse, uri);
--8<---------------cut here---------------end--------------->8---
Yes, in the case I was struggling with there is no query string, it's only
hidden parameters.
In theory I could try 1st if a query string exists. Unfortunately so far there
are no such cases.
Actually if there was a query string I'd not need to add this code block.
In order to help understand what is the expected behavior of
‘SeoContextFilter#doFilter’ and make the bug fix more future proof, it
would be really nice if you could add a unit test demonstrating how
query string examples should be handled by this fiter.
Thanks.
OK, I'll try that after finishing the stuff I was initially working on, ie
OFBIZ-9804.
Please remind me if I forget ;)
Jacques
