Skip to content

Commit

Permalink
Fixed #1948: Sanitise user input in error output
Browse files Browse the repository at this point in the history
  • Loading branch information
hylkevds committed Jun 21, 2024
1 parent 864a507 commit b8f1a83
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public static ResourcePath parsePath(ModelRegistry modelRegistry, String service
PathParser pp = new PathParser(modelRegistry, resourcePath, user.isAdmin());
pp.visit(parser.rootNode());
} catch (ParseException ex) {
throw new IllegalArgumentException("Path " + StringHelper.cleanForLogging(path) + " is not valid: " + ex.getMessage());
throw new IllegalArgumentException("Path is not valid: " + ex.getMessage());
}
return resourcePath;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ public class Service implements AutoCloseable {

private static final Logger LOGGER = LoggerFactory.getLogger(Service.class);
private static final String EXCEPTION = "Exception:";
private static final String NOT_A_VALID_PATH = "Not a valid path";
private static final String POST_ONLY_ALLOWED_TO_COLLECTIONS = "POST only allowed to Collections.";
private static final String COULD_NOT_PARSE_JSON = "Could not parse json.";
private static final String FAILED_TO_HANDLE_REQUEST_DETAILS_IN_DEBUG = "Failed to handle request (details in debug): {}";
Expand Down Expand Up @@ -375,8 +374,8 @@ private ServiceResponse handleGet(PersistenceManager pm, ServiceRequest request,
settings.getQueryDefaults().getServiceRootUrl(), version,
request.getUrlPath(),
request.getUserPrincipal());
} catch (IllegalArgumentException | IllegalStateException e) {
return errorResponse(response, 404, NOT_A_VALID_PATH + ": " + e.getMessage());
} catch (IllegalArgumentException | IllegalStateException ex) {
return errorResponse(response, 404, ex.getMessage());
}
Query query;
ResultFormatter formatter;
Expand Down Expand Up @@ -463,8 +462,8 @@ private ServiceResponse handlePost(PersistenceManager pm, String urlPath, Servic
version,
urlPath,
request.getUserPrincipal());
} catch (IllegalArgumentException | IllegalStateException e) {
return errorResponse(response, 404, NOT_A_VALID_PATH + ": " + e.getMessage());
} catch (IllegalArgumentException | IllegalStateException ex) {
return errorResponse(response, 404, ex.getMessage());
}
if (!(path.getMainElement() instanceof PathElementEntitySet)) {
return errorResponse(response, 400, POST_ONLY_ALLOWED_TO_COLLECTIONS);
Expand Down Expand Up @@ -635,8 +634,8 @@ private PathElementEntity parsePathForPutPatch(PersistenceManager pm, ServiceReq
request.getVersion(),
request.getUrlPath(),
request.getUserPrincipal());
} catch (IllegalArgumentException | IllegalStateException exc) {
throw new NoSuchEntityException(NOT_A_VALID_PATH + ": " + exc.getMessage());
} catch (IllegalArgumentException | IllegalStateException ex) {
throw new NoSuchEntityException(ex.getMessage());
}

if (!pm.validatePath(path)) {
Expand Down Expand Up @@ -728,8 +727,8 @@ private ServiceResponse executeDelete(ServiceRequest request, ServiceResponse re
request.getVersion(),
request.getUrlPath(),
request.getUserPrincipal());
} catch (IllegalArgumentException | IllegalStateException exc) {
return errorResponse(response, 404, NOT_A_VALID_PATH + ": " + exc.getMessage());
} catch (IllegalArgumentException | IllegalStateException ex) {
return errorResponse(response, 404, ex.getMessage());
}

if (path.isRef()) {
Expand Down Expand Up @@ -1017,16 +1016,17 @@ public static ServiceResponse errorResponse(ServiceResponse response, int code,
}

public static ServiceResponse jsonResponse(ServiceResponse response, String type, int code, String message) {
String cleanMessage = StringHelper.cleanForOutput(message, 200);
Map<String, Object> body = new HashMap<>();
body.put("type", type);
body.put("code", code);
body.put("message", message);
body.put("message", cleanMessage);
try {
return response.setStatus(code, JsonWriter.writeObject(body));
} catch (IOException ex) {
LOGGER.error("Failed to serialise error response.", ex);
}
return response.setStatus(code, message);
return response.setStatus(code, cleanMessage);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import net.time4j.format.expert.IsoDecimalStyle;
import net.time4j.range.MomentInterval;
import net.time4j.tz.ZonalOffset;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -63,6 +64,9 @@ public class StringHelper {

public static final Charset UTF8 = StandardCharsets.UTF_8;

private static final String OUTPUT_CLEAN_REGEX = "[^A-Za-z0-9'.,;:()?/ _-]";
private static final String OUTPUT_CLEAN_REPLACE = " ";

private static final Logger LOGGER = LoggerFactory.getLogger(StringHelper.class);
private static final String UTF8_NOT_SUPPORTED = "UTF-8 not supported?";
private static final NonZeroCondition NON_ZERO_FRACTION = new NonZeroCondition(PlainTime.NANO_OF_SECOND);
Expand Down Expand Up @@ -90,6 +94,36 @@ public static boolean isNullOrEmpty(Collection collection) {
return collection == null || collection.isEmpty();
}

/**
* Replaces characters that might cause problems in HTML and limits the
* length of the string. Currently everything that is not [A-Za-z0-9',;?/-]
* is changed to a ?.
*
* @param string The string to clean.
* @param maxLength The maximum length of string to return.
* @return The cleaned string.
*/
public static String cleanForOutput(String string, int maxLength) {
if (string == null) {
return "null";
}
return cleanForOutput(StringUtils.truncate(string, maxLength));
}

/**
* Replaces characters that might cause problems in HTML. Currently
* everything that is not [A-Za-z0-9',;?/-] is changed to a ?.
*
* @param string The string to clean.
* @return The cleaned string.
*/
public static String cleanForOutput(String string) {
if (string == null) {
return "null";
}
return string.replaceAll(OUTPUT_CLEAN_REGEX, OUTPUT_CLEAN_REPLACE);
}

/**
* Replaces characters that might break logging output. Currently: \n, \r
* and \t
Expand Down

0 comments on commit b8f1a83

Please sign in to comment.