Skip to content

Commit

Permalink
Removed untrusted data from exception messages in case they end up on…
Browse files Browse the repository at this point in the history
… the webpage
  • Loading branch information
AuroraLS3 committed Jan 15, 2023
1 parent f20a048 commit 9e11d9f
Show file tree
Hide file tree
Showing 19 changed files with 37 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import com.djrapitops.plan.delivery.web.resolver.request.Request;
import com.djrapitops.plan.delivery.web.resource.WebResource;
import com.djrapitops.plan.delivery.webserver.resolver.json.RootJSONResolver;
import com.djrapitops.plan.exceptions.connection.WebException;
import com.djrapitops.plan.exceptions.WebUserAuthException;
import com.djrapitops.plan.identification.Server;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.settings.config.paths.PluginSettings;
Expand Down Expand Up @@ -200,7 +200,7 @@ private String toJSONResourceName(String resource) {
private Optional<Response> getJSONResponse(String resource) {
try {
return jsonHandler.getResolver().resolve(new Request("GET", "/v1/" + resource, null, Collections.emptyMap()));
} catch (WebException e) {
} catch (WebUserAuthException e) {
// The rest of the exceptions should not be thrown
throw new IllegalStateException("Unexpected exception thrown: " + e, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import com.djrapitops.plan.delivery.web.resolver.request.Request;
import com.djrapitops.plan.delivery.web.resource.WebResource;
import com.djrapitops.plan.delivery.webserver.resolver.json.RootJSONResolver;
import com.djrapitops.plan.exceptions.connection.WebException;
import com.djrapitops.plan.exceptions.WebUserAuthException;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.settings.config.paths.PluginSettings;
import com.djrapitops.plan.settings.theme.Theme;
Expand Down Expand Up @@ -153,7 +153,7 @@ private String toJSONResourceName(String resource) {
private Optional<Response> getJSONResponse(String resource) {
try {
return jsonHandler.getResolver().resolve(new Request("GET", "/v1/" + resource, null, Collections.emptyMap()));
} catch (WebException e) {
} catch (WebUserAuthException e) {
// The rest of the exceptions should not be thrown
throw new IllegalStateException("Unexpected exception thrown: " + e, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import com.djrapitops.plan.delivery.web.resolver.request.Request;
import com.djrapitops.plan.delivery.web.resource.WebResource;
import com.djrapitops.plan.delivery.webserver.resolver.json.RootJSONResolver;
import com.djrapitops.plan.exceptions.connection.WebException;
import com.djrapitops.plan.exceptions.WebUserAuthException;
import com.djrapitops.plan.identification.ServerInfo;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.settings.config.paths.PluginSettings;
Expand Down Expand Up @@ -142,7 +142,7 @@ private String toJSONResourceName(String resource) {
private Optional<Response> getJSONResponse(String resource) {
try {
return jsonHandler.getResolver().resolve(new Request("GET", "/v1/" + resource, null, Collections.emptyMap()));
} catch (WebException e) {
} catch (WebUserAuthException e) {
// The rest of the exceptions should not be thrown
throw new IllegalStateException("Unexpected exception thrown: " + e.toString(), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import com.djrapitops.plan.delivery.web.resolver.Response;
import com.djrapitops.plan.delivery.web.resolver.request.Request;
import com.djrapitops.plan.delivery.webserver.resolver.json.RootJSONResolver;
import com.djrapitops.plan.exceptions.connection.WebException;
import com.djrapitops.plan.exceptions.WebUserAuthException;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.settings.config.paths.WebserverSettings;
import com.djrapitops.plan.settings.locale.LangCode;
Expand Down Expand Up @@ -179,7 +179,7 @@ private String toJsonResourceName(String resource) {
private Optional<Response> getJsonResponse(String resource) {
try {
return jsonHandler.getResolver().resolve(new Request("GET", "/v1/" + resource, null, Collections.emptyMap()));
} catch (WebException e) {
} catch (WebUserAuthException e) {
// The rest of the exceptions should not be thrown
throw new IllegalStateException("Unexpected exception thrown: " + e, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import com.djrapitops.plan.delivery.web.resolver.request.Request;
import com.djrapitops.plan.delivery.web.resource.WebResource;
import com.djrapitops.plan.delivery.webserver.resolver.json.RootJSONResolver;
import com.djrapitops.plan.exceptions.connection.WebException;
import com.djrapitops.plan.exceptions.WebUserAuthException;
import com.djrapitops.plan.identification.Server;
import com.djrapitops.plan.identification.ServerInfo;
import com.djrapitops.plan.identification.ServerUUID;
Expand Down Expand Up @@ -224,7 +224,7 @@ private String toJSONResourceName(String resource) {
private Optional<Response> getJSONResponse(String resource) {
try {
return jsonHandler.getResolver().resolve(new Request("GET", "/v1/" + resource, null, Collections.emptyMap()));
} catch (WebException e) {
} catch (WebUserAuthException e) {
// The rest of the exceptions should not be thrown
throw new IllegalStateException("Unexpected exception thrown: " + e, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class InternalErrorPage implements Page {
private final VersionChecker versionChecker;

public InternalErrorPage(
String template, String errorMsg, Throwable error,
String template, String errorMsg, @Untrusted Throwable error,
VersionChecker versionChecker
) {
this.template = template;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.djrapitops.plan.storage.database.queries.containers.ContainerFetchQueries;
import com.djrapitops.plan.storage.database.queries.objects.ServerQueries;
import com.djrapitops.plan.storage.file.PlanFiles;
import com.djrapitops.plan.utilities.dev.Untrusted;
import com.djrapitops.plan.version.VersionChecker;
import dagger.Lazy;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -219,7 +220,7 @@ public Page networkPage() throws IOException {
);
}

public Page internalErrorPage(String message, Throwable error) {
public Page internalErrorPage(String message, @Untrusted Throwable error) {
try {
return new InternalErrorPage(
getResource("error.html"), message, error,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ private Response forPage(Page page) {
.build();
}

private Response forInternalError(Throwable error, String cause) {
private Response forInternalError(@Untrusted Throwable error, String cause) {
return Response.builder()
.setMimeType(MimeType.HTML)
.setContent(pageFactory.internalErrorPage(cause, error).toHtml())
Expand Down Expand Up @@ -171,7 +171,7 @@ public Response rawPlayerPageResponse(UUID playerUUID) {
.build();
}

public Response javaScriptResponse(String fileName) {
public Response javaScriptResponse(@Untrusted String fileName) {
try {
String content = UnaryChain.of(getResource(fileName).asString())
.chain(this::replaceMainAddressPlaceholder)
Expand All @@ -189,7 +189,7 @@ public Response javaScriptResponse(String fileName) {
.setStatus(200)
.build();
} catch (UncheckedIOException e) {
return notFound404("JS File not found from jar: " + fileName + ", " + e);
return notFound404("Javascript File not found");
}
}

Expand Down Expand Up @@ -217,23 +217,23 @@ public Response cssResponse(@Untrusted String fileName) {
.setStatus(200)
.build();
} catch (UncheckedIOException e) {
return notFound404("CSS File not found from jar: " + fileName + ", " + e);
return notFound404("CSS File not found");
}
}

public Response imageResponse(String fileName) {
public Response imageResponse(@Untrusted String fileName) {
try {
return Response.builder()
.setMimeType(MimeType.IMAGE)
.setContent(getResource(fileName))
.setStatus(200)
.build();
} catch (UncheckedIOException e) {
return notFound404("Image File not found from jar: " + fileName + ", " + e);
return notFound404("Image File not found");
}
}

public Response fontResponse(String fileName) {
public Response fontResponse(@Untrusted String fileName) {
String type;
if (fileName.endsWith(".woff")) {
type = MimeType.FONT_WOFF;
Expand All @@ -252,7 +252,7 @@ public Response fontResponse(String fileName) {
.setContent(getResource(fileName))
.build();
} catch (UncheckedIOException e) {
return notFound404("Font File not found from jar: " + fileName + ", " + e);
return notFound404("Font File not found");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import com.djrapitops.plan.delivery.webserver.resolver.swagger.SwaggerJsonResolver;
import com.djrapitops.plan.delivery.webserver.resolver.swagger.SwaggerPageResolver;
import com.djrapitops.plan.exceptions.WebUserAuthException;
import com.djrapitops.plan.exceptions.connection.ForbiddenException;
import com.djrapitops.plan.utilities.dev.Untrusted;
import com.djrapitops.plan.utilities.logging.ErrorContext;
import com.djrapitops.plan.utilities.logging.ErrorLogger;
Expand Down Expand Up @@ -173,8 +172,6 @@ public Response getResponse(@Untrusted Request request) {
return tryToGetResponse(request);
} catch (NotFoundException e) {
return responseFactory.notFound404(e.getMessage());
} catch (ForbiddenException e) {
return responseFactory.forbidden403(e.getMessage());
} catch (BadRequestException e) {
return responseFactory.badRequest(e.getMessage(), request.getPath().asString());
} catch (WebUserAuthException e) {
Expand All @@ -187,7 +184,6 @@ public Response getResponse(@Untrusted Request request) {

/**
* @throws NotFoundException In some cases when page was not found, not all.
* @throws ForbiddenException If the user is not allowed to see the page
* @throws BadRequestException If the request did not have required things.
*/
private Response tryToGetResponse(@Untrusted Request request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ private InputQueryDto parseInputQueryFromQueryParams(@Untrusted Request request)
.orElseThrow(() -> new BadRequestException("'view' parameter not set (expecting json object {afterDate, afterTime, beforeDate, beforeTime})"));
return new InputQueryDto(view, queryFilters);
} catch (IOException e) {
throw new BadRequestException("Failed to decode json: '" + q + "', " + e.getMessage());
throw new BadRequestException("Failed to decode json");
}
}

Expand Down Expand Up @@ -202,7 +202,7 @@ private Response buildAndStoreResponse(InputQueryDto input, Filter.Result result
.setJSONContent(stored.json)
.build();
} catch (ParseException e) {
throw new BadRequestException("'view' date format was incorrect (expecting afterDate dd/mm/yyyy, afterTime hh:mm, beforeDate dd/mm/yyyy, beforeTime hh:mm}): " + e.getMessage());
throw new BadRequestException("'view' date format was incorrect (expecting afterDate dd/mm/yyyy, afterTime hh:mm, beforeDate dd/mm/yyyy, beforeTime hh:mm})");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public Optional<Response> resolve(Request request) {
@Untrusted String serverIdentifier = request.getQuery().get("server")
.orElseThrow(() -> new BadRequestException("Missing 'server' query parameter"));
ServerDto server = jsonFactory.serverForIdentifier(serverIdentifier)
.orElseThrow(() -> new NotFoundException("Server with identifier '" + serverIdentifier + "' was not found in the database"));
.orElseThrow(() -> new NotFoundException("Server with given identifier was not found in the database"));
return Optional.of(Response.builder()
.setJSONContent(server)
.build());
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@
package com.djrapitops.plan.exceptions;

import com.djrapitops.plan.delivery.webserver.auth.FailReason;
import com.djrapitops.plan.exceptions.connection.WebException;

/**
* Thrown when WebUser can not be authorized (WebServer).
*
* @author AuroraLS3
*/
public class WebUserAuthException extends WebException {
public class WebUserAuthException extends IllegalStateException {

private final FailReason failReason;

Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ public static Optional<Long> getTimestamp(@Untrusted Request request) {
return Optional.empty();
}
return Optional.of(timestamp);
} catch (NumberFormatException nonNumberTimestamp) {
throw new BadRequestException("'timestamp' was not a number: " + nonNumberTimestamp.getMessage());
} catch (@Untrusted NumberFormatException nonNumberTimestamp) {
throw new BadRequestException("'timestamp' was not a number");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public Filter.Result apply(@Untrusted List<InputFilterDto> filterQueries) {

private Filter.Result apply(Filter.Result current, @Untrusted InputFilterDto inputFilterDto) {
@Untrusted String kind = inputFilterDto.getKind();
Filter filter = getFilter(kind).orElseThrow(() -> new BadRequestException("Filter kind not supported: '" + kind + "'"));
Filter filter = getFilter(kind).orElseThrow(() -> new BadRequestException("Given Filter 'kind' not supported"));

return getResult(current, filter, inputFilterDto);
}
Expand All @@ -106,8 +106,7 @@ private Filter.Result getResult(Filter.Result current, Filter filter, @Untrusted
return current == null ? filter.apply(query) : current.apply(filter, query);
} catch (IllegalArgumentException badOptions) {
throw new BadRequestException("Bad parameters for filter '" + filter.getKind() +
"': expecting " + Arrays.asList(filter.getExpectedParameters()) +
", but was given " + query.getSetParameters());
"': expecting " + Arrays.asList(filter.getExpectedParameters()) + " as parameters");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ private long getTime(@Untrusted InputFilterDto query, String dateKey, String tim

try {
return dateFormat.parse(date + ' ' + time).getTime();
} catch (ParseException e) {
throw new IllegalArgumentException(e);
} catch (@Untrusted ParseException e) {
throw new IllegalArgumentException("Could not parse date from given '" + dateKey + "' and '" + timeKey + "' - expected format dd/MM/yyyy and kk:mm");
}
}
}
Loading

0 comments on commit 9e11d9f

Please sign in to comment.