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

StrictHttpFirewall: throw specialized exceptions for specific types of rejections #12191

Open
brandon1024 opened this issue Nov 10, 2022 · 0 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement

Comments

@brandon1024
Copy link

Expected Behavior

The StrictHttpFirewall should throw specialized RequestRejectedException exceptions for each type of rejection, making it easier to map rejections to appropriate HTTP response status codes by configuring RequestRejectedHandlers.

I believe the best approach would be to introduce new exceptions that extend the existing RequestRejectedException. If this approach is taken, it might be appropriate to introduce a new type of RequestRejectedHandler that accepts a generic <? extends RequestRejectedException> parameter.

private void rejectForbiddenHttpMethod(HttpServletRequest request) {
	if (this.allowedHttpMethods == ALLOW_ANY_HTTP_METHOD) {
		return;
	}
	if (!this.allowedHttpMethods.contains(request.getMethod())) {
		throw new ForbiddenHttpMethodException(
				"The request was rejected because the HTTP method \"" + request.getMethod()
						+ "\" was not included within the list of allowed HTTP methods " + this.allowedHttpMethods);
	}
}
public class ForbiddenHttpMethodException extends RequestRejectedException {
	public ForbiddenHttpMethodException(String message) {
		super(message);
	}
}
@Bean
public TypedRequestRejectedHandler<ForbiddenHttpMethodException> forbiddenMethodHandler() {
	return (request, response, exception) ->
		response.setStatus(Status.NOT_IMPLEMENTED.getStatusCode());
}

Another approach would be to introduce a flag in RequestRejectedException indicating the rejection reason, allowing RequestRejectedHandler to return the correct HTTP status according to the rejection reason of the exception.

class RequestRejectedException extends RuntimeException {
    
    private RequestRejectedReason reason;
    
    public RequestRejectedException(String message) {
        this(message, RequestRejectedReason.GENERIC);
    }

    public RequestRejectedException(String message, RequestRejectedReason reason) {
        super(message);
        this.reason = reason;
    }
    
    public RequestRejectedReason getReason() {
        return reason;
    }

    public enum RequestRejectedReason {
        FORBIDDEN_METHOD,
        BLOCKLISTED_URL,
        UNTRUSTED_HOST,
        NOT_NORMALIZED,
        NOT_PRINTABLE_ASCII_CHARS,
        INVALID_HEADER_NAME,
        INVALID_HEADER_VALUE,
        INVALID_PARAM_NAME,
        INVALID_PARAM_VALUE,
        GENERIC
    }
}

Current Behavior

The StrictHttpFirewall currently throws RequestRejectedException for all types rejections. This makes it a bit difficult to target specific types of request rejections for specific HTTP response status codes.

If we introduce a RequestRejectedHandler, the handler catches all types of rejections (forbidden methods, blocklisted URLs, untrusted hosts, etc.). If we needed to map only one rejection type (e.g. forbidden method) to a different HTTP status (404, 501, etc), we would need to introduce logic that compares against the exception message, which would be brittle.

Context

My team is developing a server implementation of the IEEE-2030.5-2018 communication protocol. To meet standardized conformance tests of the Common Smart Inverter Profile (CSIP), our application must respond with a 501 Not Implemented when an invalid HTTP method (e.g. FOO) is used.

Currently there's no elegant way to target this specific type of firewall rejection. For now, we simply register a handler that maps all rejections to a status of 501.

    @Bean
    public RequestRejectedHandler requestRejectedHandler() {
        return (request, response, exception) -> response.setStatus(Status.NOT_IMPLEMENTED.getStatusCode());
    }
@brandon1024 brandon1024 added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Nov 10, 2022
@sjohnr sjohnr added the in: web An issue in web modules (web, webmvc) label Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants