Skip to content

Commit dd1e6b9

Browse files
committed
Revisit request parameters access with HtmlUnit
Prior to this commit, the `HtmlUnitRequestBuilder` would "translate" HtmlUnit web requests into Servlet requests using APIs that were not clearly defined and meant for internal usage. HtmlUnit 2.64.0 introduced a new `.getParameters()` API for collecting `NameValuePair` parsed from the request URI or request body, depending on the nature of the request. This arrangement is much more stable and in line with HtmlUnit's expectations. This commit uses this new API and makes HtmlUnit 2.64.0 a new minimum requirement for using HtmlUnit integration with Spring Framework. This also removes tests that were previously testing HtmlUnit's behavior and using the API now marked as internal. Closes gh-28240
1 parent 10ade23 commit dd1e6b9

File tree

4 files changed

+125
-258
lines changed

4 files changed

+125
-258
lines changed

framework-platform/framework-platform.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ dependencies {
8383
api("jaxen:jaxen:1.2.0")
8484
api("junit:junit:4.13.2")
8585
api("net.sf.jopt-simple:jopt-simple:5.0.4")
86-
api("net.sourceforge.htmlunit:htmlunit:2.63.0")
86+
api("net.sourceforge.htmlunit:htmlunit:2.64.0")
8787
api("org.apache-extras.beanshell:bsh:2.0b6")
8888
api("org.apache.activemq:activemq-broker:5.16.2")
8989
api("org.apache.activemq:activemq-kahadb-store:5.16.2")

spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import java.io.File;
2020
import java.io.IOException;
21-
import java.net.URLDecoder;
2221
import java.nio.charset.Charset;
2322
import java.nio.charset.StandardCharsets;
2423
import java.nio.file.Files;
@@ -159,7 +158,7 @@ public MockHttpServletRequest buildRequest(ServletContext servletContext) {
159158
cookies(request);
160159
this.webRequest.getAdditionalHeaders().forEach(request::addHeader);
161160
locales(request);
162-
params(request, uri);
161+
params(request);
163162
request.setQueryString(uri.getQuery());
164163

165164
return postProcess(request);
@@ -364,42 +363,36 @@ private void locales(MockHttpServletRequest request) {
364363
}
365364
}
366365

367-
private void params(MockHttpServletRequest request, UriComponents uriComponents) {
368-
uriComponents.getQueryParams().forEach((name, values) -> {
369-
String urlDecodedName = urlDecode(name);
370-
values.forEach(value -> {
371-
value = (value != null ? urlDecode(value) : "");
372-
request.addParameter(urlDecodedName, value);
373-
});
374-
});
375-
for (NameValuePair param : this.webRequest.getRequestParameters()) {
376-
if (param instanceof KeyDataPair pair) {
377-
File file = pair.getFile();
378-
MockPart part;
379-
if (file != null) {
380-
part = new MockPart(pair.getName(), file.getName(), readAllBytes(file));
381-
}
382-
else {
383-
// Support empty file upload OR file upload via setData().
384-
// For an empty file upload, getValue() returns an empty string, and
385-
// getData() returns null.
386-
// For a file upload via setData(), getData() returns the file data, and
387-
// getValue() returns the file name (if set) or an empty string.
388-
part = new MockPart(pair.getName(), pair.getValue(), pair.getData());
389-
}
390-
MediaType mediaType = (pair.getMimeType() != null ? MediaType.valueOf(pair.getMimeType()) :
391-
MediaType.APPLICATION_OCTET_STREAM);
392-
part.getHeaders().setContentType(mediaType);
393-
request.addPart(part);
366+
private void params(MockHttpServletRequest request) {
367+
for (NameValuePair param : this.webRequest.getParameters()) {
368+
addRequestParameter(request, param);
369+
}
370+
}
371+
372+
private void addRequestParameter(MockHttpServletRequest request, NameValuePair param) {
373+
if (param instanceof KeyDataPair) {
374+
KeyDataPair pair = (KeyDataPair) param;
375+
File file = pair.getFile();
376+
MockPart part;
377+
if (file != null) {
378+
part = new MockPart(pair.getName(), file.getName(), readAllBytes(file));
394379
}
395380
else {
396-
request.addParameter(param.getName(), param.getValue());
381+
// Support empty file upload OR file upload via setData().
382+
// For an empty file upload, getValue() returns an empty string, and
383+
// getData() returns null.
384+
// For a file upload via setData(), getData() returns the file data, and
385+
// getValue() returns the file name (if set) or an empty string.
386+
part = new MockPart(pair.getName(), pair.getValue(), pair.getData());
397387
}
388+
MediaType mediaType = (pair.getMimeType() != null ? MediaType.valueOf(pair.getMimeType()) :
389+
MediaType.APPLICATION_OCTET_STREAM);
390+
part.getHeaders().setContentType(mediaType);
391+
request.addPart(part);
392+
}
393+
else {
394+
request.addParameter(param.getName(), param.getValue());
398395
}
399-
}
400-
401-
private String urlDecode(String value) {
402-
return URLDecoder.decode(value, StandardCharsets.UTF_8);
403396
}
404397

405398
private byte[] readAllBytes(File file) {

0 commit comments

Comments
 (0)