Hello Jacques,
jler...@apache.org writes:
> commit 5170e9d89503afa13d4ea1492b0ba73b2f92e528
> Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
> AuthorDate: Sat Nov 9 14:25:00 2019 +0100
>
> Fixed:
> (OFBIZ-)
>
> While working on OFBIZ-9804 (verification email for Newsletter) I was
> confronted
> with misc. issues. One of them was that SeoContextFilter.java was not
> handling
> query strings.
>
> This fixes it
> ---
> .../ofbiz/product/category/SeoContextFilter.java | 18
> +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git
> a/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java
>
> b/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java
> index 17ab0ae..b7cab04 100644
> ---
> a/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java
> +++
> 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.
> + 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---
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.
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
