Skip to content

Commit

Permalink
Fix #298 - Don't handle /metadata for methods other than GET
Browse files Browse the repository at this point in the history
  • Loading branch information
jamesagnew committed Feb 19, 2016
1 parent 43bdfc0 commit 6ce4056
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import ca.uhn.fhir.rest.server.SimpleBundleProvider;
import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException;

public class ConformanceMethodBinding extends BaseResourceReturningMethodBinding {

Expand Down Expand Up @@ -83,8 +84,16 @@ public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) {
return true;
}

if (theRequest.getRequestType() == RequestTypeEnum.GET && "metadata".equals(theRequest.getOperation())) {
return true;
if (theRequest.getResourceName() != null) {
return false;
}

if ("metadata".equals(theRequest.getOperation())) {
if (theRequest.getRequestType() == RequestTypeEnum.GET) {
return true;
} else {
throw new MethodNotAllowedException("/metadata request must use HTTP GET", RequestTypeEnum.GET);
}
}

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public BaseMethodBinding<?> determineResourceMethod(RequestDetails requestDetail
ResourceBinding resourceBinding = null;
BaseMethodBinding<?> resourceMethod = null;
String resourceName = requestDetails.getResourceName();
if (Constants.URL_TOKEN_METADATA.equals(resourceName) || requestType == RequestTypeEnum.OPTIONS) {
if (myServerConformanceMethod.incomingServerRequestMatchesMethod(requestDetails)) {
resourceMethod = myServerConformanceMethod;
} else if (resourceName == null) {
resourceBinding = myServerBinding;
Expand Down Expand Up @@ -1396,7 +1396,7 @@ private void writeExceptionToResponse(HttpServletResponse theResponse, BaseServe
}

private static boolean partIsOperation(String nextString) {
return nextString.length() > 0 && (nextString.charAt(0) == '_' || nextString.charAt(0) == '$');
return nextString.length() > 0 && (nextString.charAt(0) == '_' || nextString.charAt(0) == '$' || nextString.equals(Constants.URL_TOKEN_METADATA));
}

public static boolean requestIsBrowser(HttpServletRequest theRequest) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package ca.uhn.fhir.rest.server;

import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;

import java.util.concurrent.TimeUnit;

import org.apache.commons.io.IOUtils;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpRequestBase;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.dstu3.model.Patient;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;

import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.rest.annotation.ResourceParam;
import ca.uhn.fhir.rest.annotation.Validate;
import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.util.PortUtil;

public class MetadataDstu3Test2 {

private static CloseableHttpClient ourClient;
private static int ourPort;
private static Server ourServer;
private static RestfulServer servlet;
private static final FhirContext ourCtx = FhirContext.forDstu3();

@Test
public void testHttpMethods() throws Exception {
String output;

HttpRequestBase httpPost = new HttpGet("http://localhost:" + ourPort + "/metadata");
HttpResponse status = ourClient.execute(httpPost);
output = IOUtils.toString(status.getEntity().getContent());
IOUtils.closeQuietly(status.getEntity().getContent());
assertEquals(200, status.getStatusLine().getStatusCode());
assertThat(output, containsString("<Conformance"));

httpPost = new HttpPost("http://localhost:" + ourPort + "/metadata");
status = ourClient.execute(httpPost);
output = IOUtils.toString(status.getEntity().getContent());
IOUtils.closeQuietly(status.getEntity().getContent());
assertEquals(405, status.getStatusLine().getStatusCode());
assertEquals("<OperationOutcome xmlns=\"http://hl7.org/fhir\"><issue><severity value=\"error\"/><code value=\"processing\"/><diagnostics value=\"/metadata request must use HTTP GET\"/></issue></OperationOutcome>", output);

/*
* There is no @read on the RP below, so this should fail. Otherwise it
* would be interpreted as a read on ID "metadata"
*/
httpPost = new HttpGet("http://localhost:" + ourPort + "/Patient/metadata");
status = ourClient.execute(httpPost);
output = IOUtils.toString(status.getEntity().getContent());
IOUtils.closeQuietly(status.getEntity().getContent());
assertEquals(400, status.getStatusLine().getStatusCode());
}

@AfterClass
public static void afterClass() throws Exception {
ourServer.stop();
}

@BeforeClass
public static void beforeClass() throws Exception {
ourPort = PortUtil.findFreePort();
ourServer = new Server(ourPort);

DummyPatientResourceProvider patientProvider = new DummyPatientResourceProvider();

ServletHandler proxyHandler = new ServletHandler();
servlet = new RestfulServer(ourCtx);
servlet.setResourceProviders(patientProvider);
ServletHolder servletHolder = new ServletHolder(servlet);
proxyHandler.addServletWithMapping(servletHolder, "/*");
ourServer.setHandler(proxyHandler);
ourServer.start();

PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS);
HttpClientBuilder builder = HttpClientBuilder.create();
builder.setConnectionManager(connectionManager);
ourClient = builder.build();

}

public static class DummyPatientResourceProvider implements IResourceProvider {

@Validate()
public MethodOutcome validate(@ResourceParam Patient theResource) {
return new MethodOutcome();
}

@Override
public Class<Patient> getResourceType() {
return Patient.class;
}
}

}
7 changes: 7 additions & 0 deletions src/changes/changes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@
<code>ActionRequestDetails</code> argument. Thanks to Ravi Kuchi for reporting!
]]>
</action>
<action type="fix" issue="298">
<![CDATA[
Request to server at <code>[baseUrl]/metadata</code> with an HTTP method
other than GET (e.g. POST, PUT) should result in an HTTP 405. Thanks to
Michael Lawley for reporting!
]]>
</action>
</release>
<release version="1.4" date="2016-02-04">
<action type="add">
Expand Down

0 comments on commit 6ce4056

Please sign in to comment.