NettyRoutingFilter fails on decorated response with non-standard http status

[Anonymous-Coward]

Posting this here instead of SO because even if it's formulated as a question, its substance is in fact a bug, or at least a serious shortcoming/inconsistency in Spring cloud gateway, IMO.

I have a custom global filter which basically does this:

public Mono<Void> filter(final ServerWebExchange exchange, final GatewayFilterChain chain) {
	final ServerHttpResponseDecorator decoratedResponse = new ServerHttpResponseDecorator(exchange.getResponse()) {

		@Override
		public Mono<Void> writeWith(final Publisher<? extends DataBuffer> body) {
			final Flux<? extends DataBuffer> transformed = Flux.from(body).map(buffer -> {
				// do some stuff with the buffer, but don't change it
				return buffer;
			}).doOnComplete(() -> {
				LOGGER.debug("Some custom code after that");
				// ...
			});
			return super.writeWith(transformed);
		}
	};
	return chain.filter(exchange.mutate().response(decoratedResponse).build());
}

It runs before the NettyRoutingFilter, which is a global filter added by default to all http/https filter chains.

If the delegate of the decorated response has a custom status, i.e. one for which there is no HttpStatus enum member defined, NettyRoutingFilter.filter() will fail. The reason is this code in NettyRoutingFilter:

	if (status != null) {
		response.setStatusCode(status);
	}
	else if (response instanceof AbstractServerHttpResponse) {
		// https://jira.spring.io/browse/SPR-16748
		((AbstractServerHttpResponse) response)
				.setStatusCodeValue(res.status().code());
	}
	else {
		// TODO: log warning here, not throw error?
		throw new IllegalStateException("Unable to set status code on response: "
				+ res.status().code() + ", " + response.getClass());
	}

This is not trivial to test with a unit test, since it requires a full filter chain as built by spring cloud gateway at runtime. But looking at the code above, the reason should be obvious. NettyRoutingFilter can't cope with non-standard http status codes unless the response it gets is an AbstractServerHttpResponse. This is not the case if a filter running before it has already decorated the response.

I'm in a bit of a pickle/under pressure to get this sorted out. Is there a way to implement a workaround?

What I have looked at, so far:

• use a subclass of AbstractServerHttpResponse instead of a ServerHttpResponseDecorator - proved to be too difficult to implement (i.e. I've quickly gotten into abstract methods for which I didn't know what a correct implementation should do)
• replace NettyRoutingFilter with a custom implementation, which does a sane handling of the custom status by also dealing with potentially decorated responses - couldn't find a proper place where to change the configuration.

I'd be really grateful for quick feedback. I'm currently on spring-cloud-gateway 2.1.2.RELEASE, but as far as I can tell, from looking at the source code on github, the relevant code in NettyRoutingFilter is the same for all released versions.

The two abstract methods for which I don't know how to provide an implementation in a subclass of AbstractServerHttpResponse are:

	protected Mono<Void> writeWithInternal(final Publisher<? extends DataBuffer> body) {
		// TODO Auto-generated method stub
		return null;
	}

	@Override
	protected Mono<Void> writeAndFlushWithInternal(final Publisher<? extends Publisher<? extends DataBuffer>> body) {
		// TODO Auto-generated method stub
		return null;
	}

[spencergibb]

What is

sane handling of the custom status by also dealing with potentially decorated responses
?

There's no setter on the interface. See spring-projects/spring-framework#21289 (comment)

[Anonymous-Coward]

I was thinking of changing the code in NettyRoutingFilter to also check if the response is a ServerHttpResponseDecorator, and if yes, get its delegate. And do this in a loop, so multiply nested decorators are still handled properly.

Something like this in NettyRoutingFilter:

if (status != null) {
		response.setStatusCode(status);
} else {

	while (response instanceof ServerHttpResponseDecorator) {
		response = ((ServerHttpResponseDecorator) response).getDelegate();
	} 
	if (response instanceof AbstractServerHttpResponse) {
		// https://jira.spring.io/browse/SPR-16748
		((AbstractServerHttpResponse) response)
				.setStatusCodeValue(res.status().code());
	} else {
		/// throw the exception
	}
}

But is there a quick, easy workaround for how the code looks now?

[spencergibb]

PRs welcome, looks like you have something there already. Just need a test case.

Here's the implementation of AbstractServerHttpResponse that you could copy: https://github.com/spring-projects/spring-framework/blob/master/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpResponse.java

[Anonymous-Coward]

I was trying copying stuff from ReactorServerHttpResponse earlier today. Unfortunately, you can't copy much implementation from there - it relies heavily on classes which are package-private and therefore not accessible from elsewhere.

I was just thinking about a PR. How fast would this land in the public maven repos? I will have first to clear this with my current employer, though.