From 11d3ae944758eae3d487de41a328d3aa5e717ab8 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 6 Oct 2016 13:23:32 -0400 Subject: [PATCH] Fix #411 - Searching by POST with urlencoded parameters doesn't work if interceptors are accessing the parameters and there is are also parameters on the URL --- .../uhn/fhir/rest/server/RestfulServer.java | 44 ++++-- .../server/util/JaxRsResponseDstu3Test.java | 4 +- .../fhir/rest/server/SearchPostDstu3Test.java | 146 +++++++++++++++++- 3 files changed, 168 insertions(+), 26 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java index 77ec7f02feb..781a06ee062 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java @@ -528,14 +528,10 @@ public class RestfulServer extends HttpServlet implements IRestfulServer 0 && requestPath.charAt(0) == '/') { - requestPath = requestPath.substring(1); - } - - fhirServerBase = getServerBaseForRequest(theRequest); - String completeUrl; Map params = null; if (StringUtils.isNotBlank(theRequest.getQueryString())) { @@ -572,7 +560,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer 0 && requestPath.charAt(0) == '/') { + requestPath = requestPath.substring(1); + } + + fhirServerBase = getServerBaseForRequest(theRequest); + + IIdType id; populateRequestDetailsFromRequestPath(requestDetails, requestPath); diff --git a/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseDstu3Test.java b/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseDstu3Test.java index 3f614fdef8a..4663e94233f 100644 --- a/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseDstu3Test.java +++ b/hapi-fhir-jaxrsserver-base/src/test/java/ca/uhn/fhir/jaxrs/server/util/JaxRsResponseDstu3Test.java @@ -50,7 +50,7 @@ public class JaxRsResponseDstu3Test { boolean theAddContentLocationHeader = false; Response result = (Response) RestfulServerUtils.streamResponseAsResource(request.getServer(), bundle, theSummaryMode, 200, theAddContentLocationHeader, respondGzip, request); assertEquals(200, result.getStatus()); - assertEquals(Constants.CT_FHIR_JSON+Constants.CHARSET_UTF8_CTSUFFIX, result.getHeaderString(Constants.HEADER_CONTENT_TYPE)); + assertEquals(Constants.CT_FHIR_JSON_NEW+Constants.CHARSET_UTF8_CTSUFFIX, result.getHeaderString(Constants.HEADER_CONTENT_TYPE)); assertTrue(result.getEntity().toString().contains("Patient")); assertTrue(result.getEntity().toString().contains("15")); } @@ -108,7 +108,7 @@ public class JaxRsResponseDstu3Test { boolean respondGzip = true; Response result = (Response) RestfulServerUtils.streamResponseAsResource(request.getServer(), createPatient(), theSummaryMode, 200, addContentLocationHeader, respondGzip, this.request); assertEquals(200, result.getStatus()); - assertEquals("application/json+fhir; charset=UTF-8", result.getHeaderString(Constants.HEADER_CONTENT_TYPE)); + assertEquals(Constants.CT_FHIR_JSON_NEW+"; charset=UTF-8", result.getHeaderString(Constants.HEADER_CONTENT_TYPE)); assertTrue(result.getEntity().toString().contains("resourceType\": \"Patient")); assertTrue(result.getEntity().toString().contains("15")); diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchPostDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchPostDstu3Test.java index dc0e73e2a59..4ae242783cd 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchPostDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchPostDstu3Test.java @@ -2,11 +2,16 @@ package ca.uhn.fhir.rest.server; import static org.junit.Assert.assertEquals; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + import org.apache.commons.io.IOUtils; import org.apache.http.NameValuePair; import org.apache.http.client.entity.UrlEncodedFormEntity; @@ -30,16 +35,37 @@ import org.junit.Test; import com.google.common.collect.Lists; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.model.api.Bundle; +import ca.uhn.fhir.model.api.TagList; import ca.uhn.fhir.rest.annotation.OptionalParam; import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.annotation.Sort; +import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.SortSpec; +import ca.uhn.fhir.rest.method.RequestDetails; import ca.uhn.fhir.rest.param.StringAndListParam; +import ca.uhn.fhir.rest.server.SearchPostDstu3Test.ParamLoggingInterceptor; +import ca.uhn.fhir.rest.server.exceptions.AuthenticationException; +import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; +import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; +import ca.uhn.fhir.rest.server.interceptor.InterceptorAdapter; +import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.PortUtil; import ca.uhn.fhir.util.TestUtil; public class SearchPostDstu3Test { + public class ParamLoggingInterceptor extends InterceptorAdapter { + + @Override + public boolean incomingRequestPreProcessed(HttpServletRequest theRequest, HttpServletResponse theResponse) { + ourLog.info("Params: {}", theRequest.getParameterMap()); + return true; + } + + + } + private static CloseableHttpClient ourClient; private static FhirContext ourCtx = FhirContext.forDstu3(); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchPostDstu3Test.class); @@ -48,24 +74,32 @@ public class SearchPostDstu3Test { private static String ourLastMethod; private static SortSpec ourLastSortSpec; private static StringAndListParam ourLastName; + private static RestfulServer ourServlet; @Before public void before() { ourLastMethod = null; ourLastSortSpec = null; ourLastName = null; + + for (IServerInterceptor next : new ArrayList<>(ourServlet.getInterceptors())) { + ourServlet.unregisterInterceptor(next); + } } /** * See #411 */ @Test - public void testSearchWithMixedParams() throws Exception { - HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient/_search?_format=application/xml"); + public void testSearchWithMixedParamsNoInterceptorsYesParams() throws Exception { + HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient/_search?_format=application/fhir+json"); + httpPost.addHeader("Cache-Control","no-cache"); List parameters = Lists.newArrayList(); parameters.add(new BasicNameValuePair("name", "Smith")); httpPost.setEntity(new UrlEncodedFormEntity(parameters)); + ourLog.info("Outgoing post: {}", httpPost); + CloseableHttpResponse status = ourClient.execute(httpPost); try { String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); @@ -77,13 +111,110 @@ public class SearchPostDstu3Test { assertEquals(1, ourLastName.getValuesAsQueryTokens().size()); assertEquals(1, ourLastName.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().size()); assertEquals("Smith", ourLastName.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue()); - + assertEquals(Constants.CT_FHIR_JSON_NEW, status.getEntity().getContentType().getValue().replaceAll(";.*", "")); } finally { IOUtils.closeQuietly(status.getEntity().getContent()); } } + /** + * See #411 + */ + @Test + public void testSearchWithMixedParamsNoInterceptorsNoParams() throws Exception { + HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient/_search"); + httpPost.addHeader("Cache-Control","no-cache"); + List parameters = Lists.newArrayList(); + parameters.add(new BasicNameValuePair("name", "Smith")); + httpPost.setEntity(new UrlEncodedFormEntity(parameters)); + + ourLog.info("Outgoing post: {}", httpPost); + + CloseableHttpResponse status = ourClient.execute(httpPost); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(responseContent); + assertEquals(200, status.getStatusLine().getStatusCode()); + + assertEquals("search", ourLastMethod); + assertEquals(null, ourLastSortSpec); + assertEquals(1, ourLastName.getValuesAsQueryTokens().size()); + assertEquals(1, ourLastName.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().size()); + assertEquals("Smith", ourLastName.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue()); + assertEquals(Constants.CT_FHIR_XML_NEW, status.getEntity().getContentType().getValue().replaceAll(";.*", "")); + } finally { + IOUtils.closeQuietly(status.getEntity().getContent()); + } + + } + + /** + * See #411 + */ + @Test + public void testSearchWithMixedParamsYesInterceptorsYesParams() throws Exception { + ourServlet.registerInterceptor(new ParamLoggingInterceptor()); + + HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient/_search?_format=application/fhir+json"); + httpPost.addHeader("Cache-Control","no-cache"); + List parameters = Lists.newArrayList(); + parameters.add(new BasicNameValuePair("name", "Smith")); + httpPost.setEntity(new UrlEncodedFormEntity(parameters)); + + ourLog.info("Outgoing post: {}", httpPost); + + CloseableHttpResponse status = ourClient.execute(httpPost); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(responseContent); + assertEquals(200, status.getStatusLine().getStatusCode()); + + assertEquals("search", ourLastMethod); + assertEquals(null, ourLastSortSpec); + assertEquals(1, ourLastName.getValuesAsQueryTokens().size()); + assertEquals(1, ourLastName.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().size()); + assertEquals("Smith", ourLastName.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue()); + assertEquals(Constants.CT_FHIR_JSON_NEW, status.getEntity().getContentType().getValue().replaceAll(";.*", "")); + } finally { + IOUtils.closeQuietly(status.getEntity().getContent()); + } + + } + + /** + * See #411 + */ + @Test + public void testSearchWithMixedParamsYesInterceptorsNoParams() throws Exception { + ourServlet.registerInterceptor(new ParamLoggingInterceptor()); + + HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient/_search"); + httpPost.addHeader("Cache-Control","no-cache"); + List parameters = Lists.newArrayList(); + parameters.add(new BasicNameValuePair("name", "Smith")); + httpPost.setEntity(new UrlEncodedFormEntity(parameters)); + + ourLog.info("Outgoing post: {}", httpPost); + + CloseableHttpResponse status = ourClient.execute(httpPost); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(responseContent); + assertEquals(200, status.getStatusLine().getStatusCode()); + + assertEquals("search", ourLastMethod); + assertEquals(null, ourLastSortSpec); + assertEquals(1, ourLastName.getValuesAsQueryTokens().size()); + assertEquals(1, ourLastName.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().size()); + assertEquals("Smith", ourLastName.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue()); + assertEquals(Constants.CT_FHIR_XML_NEW, status.getEntity().getContentType().getValue().replaceAll(";.*", "")); + } finally { + IOUtils.closeQuietly(status.getEntity().getContent()); + } + + } + @AfterClass public static void afterClassClearContext() throws Exception { ourServer.stop(); @@ -98,11 +229,12 @@ public class SearchPostDstu3Test { DummyPatientResourceProvider patientProvider = new DummyPatientResourceProvider(); ServletHandler proxyHandler = new ServletHandler(); - RestfulServer servlet = new RestfulServer(ourCtx); - servlet.setPagingProvider(new FifoMemoryPagingProvider(10)); + ourServlet = new RestfulServer(ourCtx); + ourServlet.setDefaultResponseEncoding(EncodingEnum.XML); + ourServlet.setPagingProvider(new FifoMemoryPagingProvider(10)); - servlet.setResourceProviders(patientProvider); - ServletHolder servletHolder = new ServletHolder(servlet); + ourServlet.setResourceProviders(patientProvider); + ServletHolder servletHolder = new ServletHolder(ourServlet); proxyHandler.addServletWithMapping(servletHolder, "/*"); ourServer.setHandler(proxyHandler); ourServer.start();