Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some security issues #1397

Merged
merged 2 commits into from Jul 3, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev
Remove JSONP support
  • Loading branch information
sbrunner committed Jul 3, 2020
commit 89155f2506b9cee822e15ce60ccae390a1419d5e
4 changes: 4 additions & 0 deletions BREAKING-CHANGES.md
@@ -1,5 +1,9 @@
# Known breaking changes

## Version 3.24

- Removing JsonP support.

## Version 3.13

- If throwErrorOnExtraParameters is set to true and the JSON contains extra attributes,
Expand Down
91 changes: 15 additions & 76 deletions core/src/main/java/org/mapfish/print/servlet/MapPrinterServlet.java
Expand Up @@ -121,7 +121,7 @@ public class MapPrinterServlet extends BaseMapServlet {
/**
* If the job is done (value is true) or not (value is false).
*
* Part of the {@link #getStatus(String, String, javax.servlet.http.HttpServletRequest,
* Part of the {@link #getStatus(String, javax.servlet.http.HttpServletRequest,
* javax.servlet.http.HttpServletResponse)} response.
*/
public static final String JSON_DONE = "done";
Expand All @@ -134,22 +134,22 @@ public class MapPrinterServlet extends BaseMapServlet {
* <li>cancelled</li>
* <li>error</li>
* </ul>
* Part of the {@link #getStatus(String, String, javax.servlet.http.HttpServletRequest,
* Part of the {@link #getStatus(String, javax.servlet.http.HttpServletRequest,
* javax.servlet.http.HttpServletResponse)} response
*/
public static final String JSON_STATUS = "status";
/**
* The elapsed time in ms from the point the job started. If the job is finished, this is the duration it
* took to process the job.
*
* Part of the {@link #getStatus(String, String, javax.servlet.http.HttpServletRequest,
* Part of the {@link #getStatus(String, javax.servlet.http.HttpServletRequest,
* javax.servlet.http.HttpServletResponse)} response.
*/
public static final String JSON_ELAPSED_TIME = "elapsedTime";
/**
* A rough estimate for the time in ms the job still has to wait in the queue until it starts processing.
*
* Part of the {@link #getStatus(String, String, javax.servlet.http.HttpServletRequest,
* Part of the {@link #getStatus(String, javax.servlet.http.HttpServletRequest,
* javax.servlet.http.HttpServletResponse)} response.
*/
public static final String JSON_WAITING_TIME = "waitingTime";
Expand Down Expand Up @@ -271,7 +271,8 @@ private static PJsonObject parseJson(
private static String maybeAddRequestId(final String ref, final HttpServletRequest request) {
final Optional<String> headerName =
REQUEST_ID_HEADERS.stream().filter(h -> request.getHeader(h) != null).findFirst();
return headerName.map(s -> ref + "@" + request.getHeader(s).replaceAll("[^a-zA-Z0-9._:-]", "_")
return headerName.map(
s -> ref + "@" + request.getHeader(s).replaceAll("[^a-zA-Z0-9._:-]", "_")
).orElse(ref);
}

Expand All @@ -284,18 +285,16 @@ private static String maybeAddRequestId(final String ref, final HttpServletReque
*
* @param appId the app ID
* @param referenceId the job reference
* @param jsonpCallback if given the result is returned with a function call wrapped around it
* @param statusRequest the request object
* @param statusResponse the response object
*/
@RequestMapping(value = "/{appId}" + STATUS_URL + "/{referenceId:\\S+}.json", method = RequestMethod.GET)
public final void getStatusSpecificAppId(
@SuppressWarnings("unused") @PathVariable final String appId,
@PathVariable final String referenceId,
@RequestParam(value = "jsonp", defaultValue = "") final String jsonpCallback,
final HttpServletRequest statusRequest,
final HttpServletResponse statusResponse) {
getStatus(referenceId, jsonpCallback, statusRequest, statusResponse);
getStatus(referenceId, statusRequest, statusResponse);
}

/**
Expand All @@ -306,25 +305,21 @@ public final void getStatusSpecificAppId(
* </code></pre>
*
* @param referenceId the job reference
* @param jsonpCallback if given the result is returned with a function call wrapped around it
* @param statusRequest the request object
* @param statusResponse the response object
*/
@RequestMapping(value = STATUS_URL + "/{referenceId:\\S+}.json", method = RequestMethod.GET)
public final void getStatus(
@PathVariable final String referenceId,
@RequestParam(value = "jsonp", defaultValue = "") final String jsonpCallback,
final HttpServletRequest statusRequest,
final HttpServletResponse statusResponse) {
MDC.put(Processor.MDC_JOB_ID_KEY, referenceId);
setNoCache(statusResponse);
try {
PrintJobStatus status = this.jobManager.getStatus(referenceId);

setContentType(statusResponse, jsonpCallback);
setContentType(statusResponse);
try (PrintWriter writer = statusResponse.getWriter()) {

appendJsonpCallback(jsonpCallback, writer);
JSONWriter json = new JSONWriter(writer);
json.object();
{
Expand All @@ -339,7 +334,6 @@ public final void getStatus(
addDownloadLinkToJson(statusRequest, referenceId, json);
}
json.endObject();
appendJsonpCallbackEnd(jsonpCallback, writer);
}
} catch (JSONException | IOException e) {
throw ExceptionUtils.getRuntimeException(e);
Expand Down Expand Up @@ -638,22 +632,18 @@ public final void createReportAndGetNoAppId(
/**
* To get (in JSON) the information about the available formats and CO.
*
* @param jsonpCallback if given the result is returned with a function call wrapped around it
* @param listAppsResponse the response object
*/
@RequestMapping(value = LIST_APPS_URL, method = RequestMethod.GET)
public final void listAppIds(
@RequestParam(value = "jsonp", defaultValue = "") final String jsonpCallback,
final HttpServletResponse listAppsResponse) throws ServletException,
IOException {
MDC.remove(Processor.MDC_JOB_ID_KEY);
setCache(listAppsResponse);
Set<String> appIds = this.printerFactory.getAppIds();

setContentType(listAppsResponse, jsonpCallback);
setContentType(listAppsResponse);
try (PrintWriter writer = listAppsResponse.getWriter()) {
appendJsonpCallback(jsonpCallback, writer);

JSONWriter json = new JSONWriter(writer);
try {
json.array();
Expand All @@ -664,27 +654,23 @@ public final void listAppIds(
} catch (JSONException e) {
throw new ServletException(e);
}

appendJsonpCallbackEnd(jsonpCallback, writer);
}
}

/**
* To get (in JSON) the information about the available formats and CO.
*
* @param pretty if true then pretty print the capabilities
* @param jsonpCallback if given the result is returned with a function call wrapped around it
* @param request the request
* @param capabilitiesResponse the response object
*/
@RequestMapping(value = CAPABILITIES_URL, method = RequestMethod.GET)
public final void getCapabilities(
@RequestParam(value = "pretty", defaultValue = "false") final boolean pretty,
@RequestParam(value = "jsonp", defaultValue = "") final String jsonpCallback,
final HttpServletRequest request,
final HttpServletResponse capabilitiesResponse) throws ServletException,
IOException {
getCapabilities(DEFAULT_CONFIGURATION_FILE_KEY, pretty, jsonpCallback, request, capabilitiesResponse);
getCapabilities(DEFAULT_CONFIGURATION_FILE_KEY, pretty, request, capabilitiesResponse);
}

/**
Expand All @@ -693,15 +679,13 @@ public final void getCapabilities(
* @param appId the name of the "app" or in other words, a mapping to the configuration file for
* this request.
* @param pretty if true then pretty print the capabilities
* @param jsonpCallback if given the result is returned with a function call wrapped around it
* @param request the request
* @param capabilitiesResponse the response object
*/
@RequestMapping(value = "/{appId}" + CAPABILITIES_URL, method = RequestMethod.GET)
public final void getCapabilities(
@PathVariable final String appId,
@RequestParam(value = "pretty", defaultValue = "false") final boolean pretty,
@RequestParam(value = "jsonp", defaultValue = "") final String jsonpCallback,
final HttpServletRequest request,
final HttpServletResponse capabilitiesResponse) throws ServletException,
IOException {
Expand All @@ -719,16 +703,12 @@ public final void getCapabilities(
return;
}

setContentType(capabilitiesResponse, jsonpCallback);
setContentType(capabilitiesResponse);

final ByteArrayOutputStream prettyPrintBuffer = new ByteArrayOutputStream();

try (Writer writer = pretty ? new OutputStreamWriter(prettyPrintBuffer, Constants.DEFAULT_CHARSET) :
capabilitiesResponse.getWriter()) {
if (!pretty && !StringUtils.isEmpty(jsonpCallback)) {
writer.append(jsonpCallback).append("(");
}

JSONWriter json = new JSONWriter(writer);
try {
json.object();
Expand All @@ -749,53 +729,38 @@ public final void getCapabilities(
} catch (JSONException e) {
throw new ServletException(e);
}

if (!pretty && !StringUtils.isEmpty(jsonpCallback)) {
writer.append(");");
}
}

if (pretty) {
final JSONObject jsonObject =
new JSONObject(new String(prettyPrintBuffer.toByteArray(), Constants.DEFAULT_CHARSET));

if (!StringUtils.isEmpty(jsonpCallback)) {
capabilitiesResponse.getOutputStream().print(jsonpCallback + "(");
}
capabilitiesResponse.getOutputStream().print(jsonObject.toString(JSON_INDENT_FACTOR));
if (!StringUtils.isEmpty(jsonpCallback)) {
capabilitiesResponse.getOutputStream().print(");");
}
}
}

/**
* Get a sample request for the app. An empty response may be returned if there is not example request.
*
* @param jsonpCallback if given the result is returned with a function call wrapped around it
* @param request the request object
* @param getExampleResponse the response object
*/
@RequestMapping(value = EXAMPLE_REQUEST_URL, method = RequestMethod.GET)
public final void getExampleRequest(
@RequestParam(value = "jsonp", defaultValue = "") final String jsonpCallback,
final HttpServletRequest request,
final HttpServletResponse getExampleResponse) throws IOException {
getExampleRequest(DEFAULT_CONFIGURATION_FILE_KEY, jsonpCallback, request, getExampleResponse);
getExampleRequest(DEFAULT_CONFIGURATION_FILE_KEY, request, getExampleResponse);
}

/**
* Get a sample request for the app. An empty response may be returned if there is not example request.
*
* @param appId the id of the app to get the request for.
* @param jsonpCallback if given the result is returned with a function call wrapped around it
* @param request the request object
* @param getExampleResponse the response object
*/
@RequestMapping(value = "{appId}" + EXAMPLE_REQUEST_URL, method = RequestMethod.GET)
public final void getExampleRequest(
@PathVariable final String appId,
@RequestParam(value = "jsonp", defaultValue = "") final String jsonpCallback,
final HttpServletRequest request,
final HttpServletResponse getExampleResponse) throws
IOException {
Expand Down Expand Up @@ -827,7 +792,7 @@ public final void getExampleRequest(
jsonObject.remove(JSON_APP);
requestData = jsonObject.toString(JSON_INDENT_FACTOR);

setContentType(getExampleResponse, jsonpCallback);
setContentType(getExampleResponse);
} catch (JSONException e) {
// ignore, return raw text
}
Expand Down Expand Up @@ -864,9 +829,7 @@ public final void getExampleRequest(
}

try (PrintWriter writer = getExampleResponse.getWriter()) {
appendJsonpCallback(jsonpCallback, writer);
writer.append(result);
appendJsonpCallbackEnd(jsonpCallback, writer);
}
} catch (NoSuchAppException e) {
error(getExampleResponse, "No print app identified by: " + appId, HttpStatus.NOT_FOUND);
Expand Down Expand Up @@ -1076,31 +1039,7 @@ private <R> R loadReport(

}

private void setContentType(
final HttpServletResponse statusResponse,
final String jsonpCallback) {
if (StringUtils.isEmpty(jsonpCallback)) {
statusResponse.setContentType("application/json; charset=utf-8");
} else {
statusResponse.setContentType("application/javascript; charset=utf-8");
}
}

private void appendJsonpCallback(
final String jsonpCallback,
final PrintWriter writer) {
if (!StringUtils.isEmpty(jsonpCallback)) {
writer.append(jsonpCallback);
writer.append("(");
}
private void setContentType(final HttpServletResponse statusResponse) {
statusResponse.setContentType("application/json; charset=utf-8");
}

private void appendJsonpCallbackEnd(
final String jsonpCallback,
final PrintWriter writer) {
if (!StringUtils.isEmpty(jsonpCallback)) {
writer.append(");");
}
}

}
Expand Up @@ -110,7 +110,7 @@ public void testCreateReportAndGet_RequestAllowed_OtherGetDenied() throws Except
while (!done) {
MockHttpServletRequest servletStatusRequest = new MockHttpServletRequest("GET", statusURL);
MockHttpServletResponse servletStatusResponse = new MockHttpServletResponse();
servlet.getStatus(ref, null, servletStatusRequest, servletStatusResponse);
servlet.getStatus(ref, servletStatusRequest, servletStatusResponse);

String contentAsString = servletStatusResponse.getContentAsString();

Expand Down