search

hapifhir / hapi-fhir

🔥 HAPI FHIR - Java API for HL7 FHIR Clients and Servers
http://hapifhir.io
Apache License 2.0
1.94k stars 1.3k forks source link

Allow overriding of IncomingRequestAddressStrategy::determineServletContextPath #6038

Open alexrkopp opened 1 week ago

alexrkopp commented 1 week ago

Describe the issue I am working with a HAPI FHIR server that is fronted by a reverse proxy. The reverse proxy accepts traffic at mydomain.com/myapp/fhir/r4/QuestionnaireResponse and strips off the "/myapp" and send the resulting request to the server. It includes the not-officially-standard-but-widely-used X-Forwarded-Prefix header of "/myapp". This prefix can change and so we do not want want to require the application to be explicitly configured with a particular prefix.

We are using the spring server.forward-headers-strategy=framework property, which will correctly interpret the X-Forwarded-Prefix and reflect it appropriately in calls to request.getContextPath(). We need to get the HAPI FHIR RestfulServer to use that when parsing the incoming request. Unfortunately, https://github.com/hapifhir/hapi-fhir/blob/0397b9ddc8a8672d64fa726c26c13fbfc7b97a25/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java#L1050 is hard coded to use the IncomingRequestAddressStrategy.determineServletContextPath() static method. This ends up using server.getServletContext().getContextPath(), which returns an empty string in my use case. Ultimately, this leads to the HAPI FHIR Server being unable to properly parse the incoming requests.

It would be great if this could be overridden.

Environment (please complete the following information):

alexrkopp commented 4 days ago

I'd be happy to put together a pull request to make this configurable. I think the ideal solution would be to:

  1. Make determineServletContextPath non-static
  2. Move the existing implementation of determineServletContextPath from IncomingRequestAddressStrategy to a default implementation in IServerAddressStrategy (this should prevent a breaking change for anyone that has subclassed IncomingRequestAddressStrategy)
  3. Update RestfulServer to use myServerAddressStrategy.determineServletContextPath instead of the (previously static) IncomingRequestAddressStrategy.determineServletContextPath

Does that make sense?

jamesagnew commented 4 days ago

@alexrkopp this certainly sounds reasonable to me - please make sure to include appropriate tests which exercise the behaviour you're looking for.

jamesagnew commented 4 days ago

(oops - didn't mean to close this.. sorry!)