Skip to content

Commit

Permalink
fix(render) fixes issues when a parameter map with a null value is pa…
Browse files Browse the repository at this point in the history
…ssed into the render, causing a 404/500 error (#30944)

ref: #30942

---------

Co-authored-by: Jose Castro <[email protected]>
  • Loading branch information
wezell and jcastro-dotcms authored Dec 16, 2024
1 parent cb72fba commit aca3a06
Showing 3 changed files with 122 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
package com.dotcms.mock.request;

import java.nio.charset.Charset;
import com.google.common.collect.ImmutableMap;
import java.nio.charset.StandardCharsets;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Vector;
import java.util.stream.Collectors;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import org.apache.http.NameValuePair;
import org.apache.http.client.utils.URLEncodedUtils;
import com.dotcms.repackage.com.google.common.collect.ImmutableMap;

/**
* Mock Request Parameter using a Request Wrapper. Part of the work to be
@@ -27,8 +28,7 @@ public MockParameterRequest(HttpServletRequest request) {
public MockParameterRequest(HttpServletRequest request, Map<String, String> setMe) {
super(request);
HashMap<String, String> mutable = new HashMap<>();

List<NameValuePair> additional = URLEncodedUtils.parse(request.getQueryString(), Charset.forName("UTF-8"));
List<NameValuePair> additional = URLEncodedUtils.parse(request.getQueryString(), StandardCharsets.UTF_8);
for(NameValuePair nvp : additional) {
mutable.put(nvp.getName(),nvp.getValue());
}
@@ -40,7 +40,7 @@ public MockParameterRequest(HttpServletRequest request, Map<String, String> setM
mutable.put(key, request.getParameter(key));
}
mutable.putAll(setMe);

mutable.values().removeIf(Objects::isNull);
params = ImmutableMap.copyOf(mutable);
}

Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
package com.dotcms.vanityurl.filters;

import static com.dotmarketing.filters.Constants.CMS_FILTER_QUERY_STRING_OVERRIDE;
import static com.dotmarketing.filters.Constants.CMS_FILTER_URI_OVERRIDE;

import com.dotcms.vanityurl.model.VanityUrlResult;
import com.dotmarketing.util.UtilMethods;
import com.google.common.collect.ImmutableMap;
import com.liferay.util.StringPool;
import org.apache.http.NameValuePair;
import org.apache.http.client.utils.URLEncodedUtils;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import org.apache.http.NameValuePair;
import org.apache.http.client.utils.URLEncodedUtils;

import static com.dotmarketing.filters.Constants.CMS_FILTER_QUERY_STRING_OVERRIDE;
import static com.dotmarketing.filters.Constants.CMS_FILTER_URI_OVERRIDE;

/**
* The VanityUrlOverrideRequest merges the parameters set in the original request and merges them
@@ -37,7 +38,7 @@ public VanityUrlRequestWrapper(final HttpServletRequest request, final VanityUrl
final boolean vanityHasQueryString = UtilMethods.isSet(vanityUrlResult.getQueryString());

final StringBuilder params = new StringBuilder();
params.append(request.getQueryString());
params.append(UtilMethods.isSet(request.getQueryString()) ? request.getQueryString() : StringPool.BLANK);
final Map<String, String> vanityParams = convertURLParamsStringToMap(vanityUrlResult.getQueryString());
final Map<String, String> requestParams = convertURLParamsStringToMap(request.getQueryString());
if(vanityHasQueryString){
@@ -46,32 +47,26 @@ public VanityUrlRequestWrapper(final HttpServletRequest request, final VanityUrl
final String value = entry.getValue();
//add to the request.getQueryString() the vanity parameters that are not already present, the key and value must not be the same
if(!requestParams.containsKey(key) || !requestParams.get(key).equals(value)){
params.append("&" + key + "=" + value);
params.append(StringPool.AMPERSAND).append(key).append(StringPool.EQUAL).append(value);
}
}
}
this.newQueryString = params.toString();



// we create a new map here because it merges the
Map<String,String[]> tempMap = new HashMap<>(request.getParameterMap());
// we create a new map here because it merges the
final Map<String,String[]> tempMap = new HashMap<>(request.getParameterMap());
if(vanityHasQueryString) {
List<NameValuePair> additional = URLEncodedUtils.parse(newQueryString, StandardCharsets.UTF_8);
for(NameValuePair nvp : additional) {
final List<NameValuePair> additional = URLEncodedUtils.parse(newQueryString, StandardCharsets.UTF_8);
for (final NameValuePair nvp : additional) {
tempMap.compute(nvp.getName(), (k, v) -> (v == null) ? new String[] {nvp.getValue()} : new String[]{nvp.getValue(),v[0]});
}
}


this.queryParamMap = ImmutableMap.copyOf(tempMap);

this.responseCode = vanityUrlResult.getResponseCode();
request.setAttribute(CMS_FILTER_URI_OVERRIDE, vanityUrlResult.getRewrite());
request.setAttribute(CMS_FILTER_QUERY_STRING_OVERRIDE, this.newQueryString);
this.setAttribute(CMS_FILTER_URI_OVERRIDE, vanityUrlResult.getRewrite());
this.setAttribute(CMS_FILTER_QUERY_STRING_OVERRIDE, this.newQueryString);

}

/**
103 changes: 103 additions & 0 deletions src/test/java/com/dotcms/mock/request/MockParameterRequestTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package com.dotcms.mock.request;

import java.util.HashMap;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import javax.servlet.http.HttpServletRequest;
import java.util.Collections;
import java.util.Enumeration;
import java.util.Map;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

public class MockParameterRequestTest {



HttpServletRequest getMockRequest() {
return Mockito.mock(HttpServletRequest.class);
}
@Test
void getParameter_returnsCorrectValue() {

HttpServletRequest mockRequest = getMockRequest();

Mockito.when(mockRequest.getQueryString()).thenReturn("param2=value2");

MockParameterRequest mockParameterRequest = new MockParameterRequest(mockRequest);

assertEquals("value2", mockParameterRequest.getParameter("param2"));
}

@Test
void getParameter_returnsNullForNonExistentParameter() {
HttpServletRequest mockRequest = getMockRequest();
Mockito.when(mockRequest.getQueryString()).thenReturn("");

MockParameterRequest mockParameterRequest = new MockParameterRequest(mockRequest);

assertNull(mockParameterRequest.getParameter("nonExistentParam"));
}

@Test
void getParameterNames_returnsAllParameterNames() {
HttpServletRequest mockRequest = getMockRequest();

Mockito.when(mockRequest.getQueryString()).thenReturn("param1=value1");

MockParameterRequest mockParameterRequest = new MockParameterRequest(mockRequest);

Enumeration<String> parameterNames = mockParameterRequest.getParameterNames();
assertEquals(true, parameterNames.hasMoreElements());
assertEquals("param1", parameterNames.nextElement());
assertEquals(false, parameterNames.hasMoreElements());
}

@Test
void getParameterMap_returnsCorrectParameterMap() {
HttpServletRequest mockRequest = getMockRequest();
Mockito.when(mockRequest.getParameterNames()).thenReturn(Collections.enumeration(Collections.singleton("param1")));
Mockito.when(mockRequest.getParameter("param1")).thenReturn("value1");
Mockito.when(mockRequest.getQueryString()).thenReturn("param1=value1");

MockParameterRequest mockParameterRequest = new MockParameterRequest(mockRequest);

Map<String, String[]> parameterMap = mockParameterRequest.getParameterMap();
assertEquals(1, parameterMap.size());
assertEquals("value1", parameterMap.get("param1")[0]);
}

@Test
void getParameterMap_handlesEmptyQueryString() {
HttpServletRequest mockRequest = getMockRequest();
Mockito.when(mockRequest.getQueryString()).thenReturn("");

MockParameterRequest mockParameterRequest = new MockParameterRequest(mockRequest);

Map<String, String[]> parameterMap = mockParameterRequest.getParameterMap();
assertEquals(0, parameterMap.size());
}



@Test
void test_when_null_paramter_is_passed_in() {

HttpServletRequest mockRequest = getMockRequest();

Mockito.when(mockRequest.getQueryString()).thenReturn("param2=value2");
Map<String,String> badMap = new HashMap<>();

badMap.put("badParam", null);
MockParameterRequest mockParameterRequest = new MockParameterRequest(mockRequest, badMap);

assertEquals("value2", mockParameterRequest.getParameter("param2"));
assertNull(mockParameterRequest.getParameter("badParam"));
}



}

0 comments on commit aca3a06

Please sign in to comment.