Issue #12720 - fix servlet error dispatch for errors generated by SecurityHandler

[lachlan-roberts]

Closes #12720

Currently SecurityHandler does only Response.writeError when a 403 response needs to be sent. However this will not dispatch to a servlet error page, for this we need to do AuthenticationState.writeError and handle the case if it returns AuthenticationState.ServeAs.

[gregw]

@lachlan-roberts the CI failures look to be more than flakes.

[gregw]

more than flakes

[lachlan-roberts]

@gregw the tests are failing because the AuthenticationState.writeError and ErrorHandler.writeError do not support taking a message string.

So do you think i should adjust both the AuthenticationState.writeError and ErrorHandler.writeError API to take a String message, or should I adjust the tests to not check for the !authroized string?

[gregw]

This can return the unwrapped response, which will provide the unwrapped request. So either only wrap the request in the if statement above OR you always need to wrap the response to return the wrapped request. See above suggestion

[gregw]

Suggested change

	HttpURI uri = request.getHttpURI();
	Request wrappedRequest = serveAs.wrap(request);
	if (!uri.equals(wrappedRequest.getHttpURI()))
	{
	// URI is replaced, so filter out all metadata for the old URI
	response.getHeaders().put(HttpHeader.CACHE_CONTROL.asString(), HttpHeaderValue.NO_CACHE.asString());
	response.getHeaders().putDate(HttpHeader.EXPIRES.asString(), 1);
	HttpFields.Mutable headers = new HttpFields.Mutable.Wrapper(response.getHeaders())
	{
	@Override
	public HttpField onAddField(HttpField field)
	{
	if (field.getHeader() == null)
	return field;
	return switch (field.getHeader())
	{
	case CACHE_CONTROL, PRAGMA, ETAG, EXPIRES, LAST_MODIFIED, AGE -> null;
	default -> field;
	};
	}
	};
	return new Response.Wrapper(wrappedRequest, response)
	{
	@Override
	public HttpFields.Mutable getHeaders()
	{
	return headers;
	}
	@Override
	public Request getRequest()
	{
	return wrappedRequest;
	}
	};
	}
	HttpURI uri = request.getHttpURI();
	Request wrappedRequest = serveAs.wrap(request);
	HttpFields.Mutable headers;
	if (uri.equals(wrappedRequest.getHttpURI()))
	{
	headers = response.getHeaders()
	}
	else
	{
	// URI is replaced, so filter out all metadata for the old URI
	response.getHeaders().put(HttpHeader.CACHE_CONTROL.asString(), HttpHeaderValue.NO_CACHE.asString());
	response.getHeaders().putDate(HttpHeader.EXPIRES.asString(), 1);
	headers = new HttpFields.Mutable.Wrapper(response.getHeaders())
	{
	@Override
	public HttpField onAddField(HttpField field)
	{
	if (field.getHeader() == null)
	return field;
	return switch (field.getHeader())
	{
	case CACHE_CONTROL, PRAGMA, ETAG, EXPIRES, LAST_MODIFIED, AGE -> null;
	default -> field;
	};
	}
	};
	}
	return new Response.Wrapper(wrappedRequest, response)
	{
	@Override
	public HttpFields.Mutable getHeaders()
	{
	return headers;
	}
	@Override
	public Request getRequest()
	{
	return wrappedRequest;
	}
	};
	}