REST "next" and "previous" links can return incorrect protocal
Description
Activity
Ian Bacher February 21, 2025 at 8:05 PM
Yes, exactly. It makes sense to put this in REST, but it might even be nice as a utility method in Core (I think there’s a WebUtils class or something similar).
Mark Goodrich February 21, 2025 at 6:45 PM
Yes, it looks like we basically want to make a utility method out of this block of code?
String scheme;
if (StringUtils.isNotBlank(request.getHeader("X-Forwarded-Proto"))) {
scheme = request.getHeader("X-Forwarded-Proto");
} else {
scheme = request.getScheme();
}
String host;
if (StringUtils.isNotBlank(request.getHeader("X-Forwarded-Host"))) {
host = request.getHeader("X-Forwarded-Host");
} else {
host = request.getServerName();
}
int serverPort = request.getServerPort();
String port;
if (StringUtils.isNotBlank(request.getHeader("X-Forwarded-Port"))) {
port = ":" + request.getHeader("X-Forwarded-Port");
if (scheme.equalsIgnoreCase("http") && port.equals(":80")) {
port = "";
} else if (scheme.equalsIgnoreCase("https") && port.equals(":443")) {
port = "";
}
} else if ("http".equalsIgnoreCase(request.getScheme())) {
port = serverPort == 80 ? "" : ":" + serverPort;
} else if ("https".equalsIgnoreCase(request.getScheme())) {
port = serverPort == 443 ? "" : ":" + serverPort;
} else {
port = ":" + serverPort;
}
FhirVersionUtils.FhirVersion fhirVersion = FhirVersionUtils.getFhirVersion(request);
return scheme + "://" + host + port + StringUtils.defaultString(request.getContextPath())Ian Bacher February 21, 2025 at 4:37 PM
So, to address the same thing in FHIR2, we introduced this class (an AddressStrategy is a HAPI thing, but it’s just pulling things out of the HttpServletRequest headers). It relies on semi-standardized headers most proxy’s either should send or can be configured to send (for, e.g., our O3-hosted systems there are actually two separate nginx proxies in front of Tomcat).
The one thing it doesn’t handle is the now-standardized Forwarded header, mostly because there’s no built-in for that in nginx.
And there are definitely a few things in there that can be skipped (as a fallback, we introduced a GP so implementations could manually set what the correct URL prefix is and our requests are ultimately forwarded from /ws/fhir2/R4 to /ms/fhir2Servlet, but I we wanted the links to read /ws/fhir2/R4.
Mark Goodrich February 20, 2025 at 11:16 PM
Assumedly we want to generate the protocol and server name ourselves (which we already stored somewhere, I believe) and then append the serlvet path and query string.
Details
Assignee
UnassignedUnassignedReporter
Mark GoodrichMark GoodrichPriority
Low
Details
Details
Assignee
Reporter
Priority
Low
When generating the “next” and “previous” links for a REST request, the module relies on the getRequestUrl() method:
https://github.com/openmrs/openmrs-module-webservices.rest/blob/master/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/RequestContext.java#L173
However, when the OpenMRS server is sitting behind a load balancer (or potentially behind a reverse-proxy) the generated protocol may be incorrect… specifically the URL may use the “http” protocol instead of “https”.
See:
https://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServletRequest.html#getRequestURL()
https://stackoverflow.com/questions/29469929/why-does-request-getrequesturl-return-non-https-url
We should have more robust method to generate the URL so as to not encounter this issue.
(Thoughts @Ian Bacher @Mike Seaton … we are running into this problem on a Prod server, assumedly because we are serving them behind Caddy which is operating as a reverse proxy).