diff --git a/src/main/java/org/htmlunit/HttpWebConnection.java b/src/main/java/org/htmlunit/HttpWebConnection.java index 1ee1aae6d2..741a1be735 100644 --- a/src/main/java/org/htmlunit/HttpWebConnection.java +++ b/src/main/java/org/htmlunit/HttpWebConnection.java @@ -60,10 +60,8 @@ import org.apache.http.client.CredentialsProvider; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpDelete; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpHead; -import org.apache.http.client.methods.HttpOptions; import org.apache.http.client.methods.HttpPatch; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; @@ -302,8 +300,12 @@ private HttpUriRequest makeHttpMethod(final WebRequest webRequest, final HttpCli final HttpEntityEnclosingRequest method = (HttpEntityEnclosingRequest) httpMethod; - if (webRequest.getEncodingType() == FormEncodingType.URL_ENCODED && method instanceof HttpPost) { - final HttpPost postMethod = (HttpPost) method; + if (webRequest.getEncodingType() == FormEncodingType.URL_ENCODED + && (method instanceof HttpPost + || method instanceof HttpPatch + || method instanceof HttpPut + || method instanceof org.htmlunit.httpclient.HttpDelete + || method instanceof org.htmlunit.httpclient.HttpOptions)) { if (webRequest.getRequestBody() == null) { final List pairs = webRequest.getRequestParameters(); final String query = HttpUtils.toQueryFormFields(pairs, charset); @@ -318,17 +320,21 @@ private HttpUriRequest makeHttpMethod(final WebRequest webRequest, final HttpCli urlEncodedEntity = new StringEntity(query, charset); urlEncodedEntity.setContentType(URLEncodedUtils.CONTENT_TYPE); } - postMethod.setEntity(urlEncodedEntity); + method.setEntity(urlEncodedEntity); } else { final String body = StringUtils.defaultString(webRequest.getRequestBody()); final StringEntity urlEncodedEntity = new StringEntity(body, charset); urlEncodedEntity.setContentType(URLEncodedUtils.CONTENT_TYPE); - postMethod.setEntity(urlEncodedEntity); + method.setEntity(urlEncodedEntity); } } - else if (webRequest.getEncodingType() == FormEncodingType.TEXT_PLAIN && method instanceof HttpPost) { - final HttpPost postMethod = (HttpPost) method; + else if (webRequest.getEncodingType() == FormEncodingType.TEXT_PLAIN + && (method instanceof HttpPost + || method instanceof HttpPatch + || method instanceof HttpPut + || method instanceof org.htmlunit.httpclient.HttpDelete + || method instanceof org.htmlunit.httpclient.HttpOptions)) { if (webRequest.getRequestBody() == null) { final StringBuilder body = new StringBuilder(); for (final NameValuePair pair : webRequest.getRequestParameters()) { @@ -339,13 +345,13 @@ else if (webRequest.getEncodingType() == FormEncodingType.TEXT_PLAIN && method i } final StringEntity bodyEntity = new StringEntity(body.toString(), charset); bodyEntity.setContentType(MimeType.TEXT_PLAIN); - postMethod.setEntity(bodyEntity); + method.setEntity(bodyEntity); } else { final String body = StringUtils.defaultString(webRequest.getRequestBody()); final StringEntity bodyEntity = new StringEntity(body, ContentType.create(MimeType.TEXT_PLAIN, charset)); - postMethod.setEntity(bodyEntity); + method.setEntity(bodyEntity); } } else if (FormEncodingType.MULTIPART == webRequest.getEncodingType()) { @@ -364,7 +370,7 @@ else if (FormEncodingType.MULTIPART == webRequest.getEncodingType()) { } method.setEntity(builder.build()); } - else { // for instance a PUT or PATCH request + else { // for instance a PATCH request final String body = webRequest.getRequestBody(); if (body != null) { method.setEntity(new StringEntity(body, charset)); @@ -507,11 +513,11 @@ private static HttpRequestBase buildHttpMethod(final HttpMethod submitMethod, fi break; case DELETE: - method = new HttpDelete(uri); + method = new org.htmlunit.httpclient.HttpDelete(uri); break; case OPTIONS: - method = new HttpOptions(uri); + method = new org.htmlunit.httpclient.HttpOptions(uri); break; case HEAD: diff --git a/src/main/java/org/htmlunit/WebRequest.java b/src/main/java/org/htmlunit/WebRequest.java index fcbb59b0c2..e1764280f5 100644 --- a/src/main/java/org/htmlunit/WebRequest.java +++ b/src/main/java/org/htmlunit/WebRequest.java @@ -366,14 +366,22 @@ public List getParameters() { if (HttpMethod.POST != getHttpMethod() && HttpMethod.PUT != getHttpMethod() && HttpMethod.PATCH != getHttpMethod()) { + final List allParameters = new ArrayList<>(); + allParameters.addAll(HttpUtils.parseUrlQuery(getUrl().getQuery(), getCharset())); + + // a bit strange but in case of GET, TRACE, DELETE, OPTIONS and HEAD + // HttpWebConnection.makeHttpMethod() moves the parameters up to the query + // to reflect this we have to take the parameters into account even if this + // looks wrong for GET requests if (!getRequestParameters().isEmpty()) { return normalize(getRequestParameters()); } - return normalize(HttpUtils.parseUrlQuery(getUrl().getQuery(), getCharset())); + return normalize(allParameters); } - if (getEncodingType() == FormEncodingType.URL_ENCODED && HttpMethod.POST == getHttpMethod()) { + if (getEncodingType() == FormEncodingType.URL_ENCODED + && (HttpMethod.POST == getHttpMethod() || HttpMethod.PUT == getHttpMethod())) { if (getRequestBody() == null) { final List allParameters = new ArrayList<>(); allParameters.addAll(HttpUtils.parseUrlQuery(getUrl().getQuery(), getCharset())); @@ -388,7 +396,8 @@ public List getParameters() { return normalize(allParameters); } - if (getEncodingType() == FormEncodingType.TEXT_PLAIN && HttpMethod.POST == getHttpMethod()) { + if (getEncodingType() == FormEncodingType.TEXT_PLAIN + && (HttpMethod.POST == getHttpMethod() || HttpMethod.PUT == getHttpMethod())) { if (getRequestBody() == null) { final List allParameters = new ArrayList<>(); allParameters.addAll(HttpUtils.parseUrlQuery(getUrl().getQuery(), getCharset())); @@ -401,7 +410,7 @@ public List getParameters() { } if ((getEncodingType() == FormEncodingType.URL_ENCODED || getEncodingType() == FormEncodingType.TEXT_PLAIN) - && (HttpMethod.PUT == getHttpMethod() || HttpMethod.PATCH == getHttpMethod())) { + && HttpMethod.PATCH == getHttpMethod()) { return normalize(HttpUtils.parseUrlQuery(getUrl().getQuery(), getCharset())); } @@ -487,8 +496,12 @@ public void setRequestBody(final String requestBody) throws RuntimeException { + "the two are mutually exclusive!"; throw new RuntimeException(msg); } - if (httpMethod_ != HttpMethod.POST && httpMethod_ != HttpMethod.PUT && httpMethod_ != HttpMethod.PATCH) { - final String msg = "The request body may only be set for POST, PUT or PATCH requests!"; + if (httpMethod_ != HttpMethod.POST + && httpMethod_ != HttpMethod.PUT + && httpMethod_ != HttpMethod.PATCH + && httpMethod_ != HttpMethod.DELETE + && httpMethod_ != HttpMethod.OPTIONS) { + final String msg = "The request body may only be set for POST, PUT, PATCH, DELETE or OPTIONS requests!"; throw new RuntimeException(msg); } requestBody_ = requestBody; diff --git a/src/main/java/org/htmlunit/httpclient/HttpDelete.java b/src/main/java/org/htmlunit/httpclient/HttpDelete.java new file mode 100644 index 0000000000..5463dfb174 --- /dev/null +++ b/src/main/java/org/htmlunit/httpclient/HttpDelete.java @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2002-2024 Gargoyle Software Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.htmlunit.httpclient; + +import java.net.URI; + +import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; + +/** + * Customized HttpDelete for HtmlUnit to be an HttpEntityEnclosingRequestBase. + * + * @author Ronald Brill + */ +public class HttpDelete extends HttpEntityEnclosingRequestBase { + + public HttpDelete() { + super(); + } + + public HttpDelete(final URI uri) { + super(); + setURI(uri); + } + + /** + * @param uri the uri + * @throws IllegalArgumentException if the uri is invalid. + */ + public HttpDelete(final String uri) { + super(); + setURI(URI.create(uri)); + } + + @Override + public String getMethod() { + return org.apache.http.client.methods.HttpDelete.METHOD_NAME; + } +} diff --git a/src/main/java/org/htmlunit/httpclient/HttpOptions.java b/src/main/java/org/htmlunit/httpclient/HttpOptions.java new file mode 100644 index 0000000000..723fa53afa --- /dev/null +++ b/src/main/java/org/htmlunit/httpclient/HttpOptions.java @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2002-2024 Gargoyle Software Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.htmlunit.httpclient; + +import java.net.URI; + +import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; + +/** + * Customized HttpOptions for HtmlUnit to be an HttpEntityEnclosingRequestBase. + * + * @author Ronald Brill + */ +public class HttpOptions extends HttpEntityEnclosingRequestBase { + + public HttpOptions() { + super(); + } + + public HttpOptions(final URI uri) { + super(); + setURI(uri); + } + + /** + * @param uri the uri + * @throws IllegalArgumentException if the uri is invalid. + */ + public HttpOptions(final String uri) { + super(); + setURI(URI.create(uri)); + } + + @Override + public String getMethod() { + return org.apache.http.client.methods.HttpOptions.METHOD_NAME; + } +} diff --git a/src/main/java/org/htmlunit/javascript/host/xml/XMLHttpRequest.java b/src/main/java/org/htmlunit/javascript/host/xml/XMLHttpRequest.java index 1f5df76712..8194babdeb 100644 --- a/src/main/java/org/htmlunit/javascript/host/xml/XMLHttpRequest.java +++ b/src/main/java/org/htmlunit/javascript/host/xml/XMLHttpRequest.java @@ -806,7 +806,9 @@ private void prepareRequestContent(final Object content) { if (content != null && (HttpMethod.POST == webRequest_.getHttpMethod() || HttpMethod.PUT == webRequest_.getHttpMethod() - || HttpMethod.PATCH == webRequest_.getHttpMethod()) + || HttpMethod.PATCH == webRequest_.getHttpMethod() + || HttpMethod.DELETE == webRequest_.getHttpMethod() + || HttpMethod.OPTIONS == webRequest_.getHttpMethod()) && !JavaScriptEngine.isUndefined(content)) { final boolean setEncodingType = webRequest_.getAdditionalHeader(HttpHeader.CONTENT_TYPE) == null; diff --git a/src/test/java/org/htmlunit/MiniServer.java b/src/test/java/org/htmlunit/MiniServer.java index fcc98c9c6d..a36cdc8d2a 100644 --- a/src/test/java/org/htmlunit/MiniServer.java +++ b/src/test/java/org/htmlunit/MiniServer.java @@ -53,6 +53,7 @@ public class MiniServer extends Thread implements Closeable { private final AtomicBoolean started_ = new AtomicBoolean(false); private final MockWebConnection mockWebConnection_; private volatile ServerSocket serverSocket_; + private String lastRequest_; private static final Set DROP_REQUESTS = new HashSet<>(); private static final Set DROP_GET_REQUESTS = new HashSet<>(); @@ -201,6 +202,7 @@ else if ("POST".equalsIgnoreCase(methodText)) { } try { final URL url = new URL("http://localhost:" + port_ + requestedPath); + lastRequest_ = request; return new WebRequest(url, submitMethod); } catch (final MalformedURLException e) { @@ -209,6 +211,10 @@ else if ("POST".equalsIgnoreCase(methodText)) { } } + public String getLastRequest() { + return lastRequest_; + } + /** * ShutDown this server. * @throws InterruptedException in case of error diff --git a/src/test/java/org/htmlunit/WebRequest2Test.java b/src/test/java/org/htmlunit/WebRequest2Test.java index 0ecc35746f..d13f15bf6e 100644 --- a/src/test/java/org/htmlunit/WebRequest2Test.java +++ b/src/test/java/org/htmlunit/WebRequest2Test.java @@ -198,24 +198,25 @@ public void test() throws Exception { request.setHttpMethod(httpMethod_); request.setEncodingType(encoding_); - if (parameter_ != null) { - request.setRequestParameters(parameter_.getPairs()); + if (body_ == null) { + if (parameter_ != null) { + request.setRequestParameters(parameter_.getPairs()); + } } - - if (body_ != null) { - try { + else { + if (httpMethod_ == HttpMethod.POST + || httpMethod_ == HttpMethod.PUT + || httpMethod_ == HttpMethod.PATCH + || httpMethod_ == HttpMethod.DELETE + || httpMethod_ == HttpMethod.OPTIONS) { request.setRequestBody(body_); } - catch (final RuntimeException e) { - // ignore - } } - final Page page = getWebClient().getPage(request); - assertTrue(page instanceof TextPage); + final WebResponse response = getWebClient().getWebConnection().getResponse(request); // calculate expectation from bounce servlet - String expectedContent = ((TextPage) page).getContent(); + String expectedContent = response.getContentAsString(); if (HttpMethod.HEAD.equals(request.getHttpMethod())) { assertEquals(0, expectedContent.length()); diff --git a/src/test/java/org/htmlunit/javascript/host/xml/XMLHttpRequestSentContentTest.java b/src/test/java/org/htmlunit/javascript/host/xml/XMLHttpRequestSentContentTest.java new file mode 100644 index 0000000000..17957f254d --- /dev/null +++ b/src/test/java/org/htmlunit/javascript/host/xml/XMLHttpRequestSentContentTest.java @@ -0,0 +1,140 @@ +/* + * Copyright (c) 2002-2024 Gargoyle Software Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.htmlunit.javascript.host.xml; + +import java.net.URL; + +import org.htmlunit.MiniServer; +import org.htmlunit.MockWebConnection; +import org.htmlunit.WebDriverTestCase; +import org.htmlunit.WebTestCase; +import org.htmlunit.junit.BrowserRunner; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.openqa.selenium.WebDriver; + +/** + * Tests for the content sent to the server. + * + * @author Ronald Brill + * + */ +@RunWith(BrowserRunner.class) +public final class XMLHttpRequestSentContentTest extends WebDriverTestCase { + + @Before + public void before() throws Exception { + // Chrome seems to cache preflight results + shutDownAll(); + MiniServer.resetDropRequests(); + } + + @After + public void after() throws Exception { + MiniServer.resetDropRequests(); + } + + /** + * @throws Exception if the test fails + */ + @Test + public void getShouldNotSendBody() throws Exception { + testSendBody("GET", false); + } + + /** + * @throws Exception if the test fails + */ + @Test + public void postShouldSendBody() throws Exception { + testSendBody("POST", true); + } + + /** + * @throws Exception if the test fails + */ + @Test + public void putShouldSendBody() throws Exception { + testSendBody("PUT", true); + } + + /** + * @throws Exception if the test fails + */ + @Test + public void patchShouldSendBody() throws Exception { + testSendBody("PATCH", true); + } + + /** + * @throws Exception if the test fails + */ + @Test + public void deleteShouldSendBody() throws Exception { + testSendBody("DELETE", true); + } + + /** + * @throws Exception if the test fails + */ + @Test + public void optionsShouldSendBody() throws Exception { + testSendBody("OPTIONS", true); + } + + /** + * @throws Exception if the test fails + */ + @Test + public void headShouldNotSendBody() throws Exception { + testSendBody("HEAD", false); + } + + private void testSendBody(final String method, final boolean bodyIncluded) throws Exception { + final String html = ""; + + final MockWebConnection mockWebConnection = getMockWebConnection(); + mockWebConnection.setResponse(WebTestCase.URL_FIRST, html); + mockWebConnection.setDefaultResponse(""); + + try (MiniServer miniServer = new MiniServer(PORT, mockWebConnection)) { + miniServer.start(); + + final WebDriver driver = getWebDriver(); + driver.get(WebTestCase.URL_FIRST.toExternalForm()); + + assertEquals(2, mockWebConnection.getRequestCount()); + assertEquals(new URL(WebTestCase.URL_FIRST, "second.html?a=x"), + mockWebConnection.getLastWebRequest().getUrl()); + + if (bodyIncluded) { + assertTrue(miniServer.getLastRequest(), miniServer.getLastRequest().contains("\nx=body")); + } + else { + assertTrue(miniServer.getLastRequest(), !miniServer.getLastRequest().contains("\nx=body")); + } + } + } +}