spring-framework icon indicating copy to clipboard operation
spring-framework copied to clipboard

Support HTTP trailing headers

Open bmaassenee opened this issue 1 year ago • 4 comments

With the current WebClient implementation its not possible to get the trailer data of a chunked http response. For example for the response:

HTTP/1.1 200 OK
Transfer-Encoding: chunked
Content-Type: text/binary
Connection: Keep-Alive
Keep-Alive: timeout-20
Date: Fri, 03 May 2024 14:26:43 GMT
Cache-Control: max-age=1
x-ee-cameraid: 100f9718
accept-ranges: bytes
trailer: x-result,x-error

0
x-result: 1
x-error: Failed to determine asset size

The ResponseEntity object has no way to access the trailer. The data you get from the body doesn't contain the trailer info, and neither does the headers object. I debugged the issue a little and found that netty correctly parses and makes the data available: image

But the info is "thrown away" in the reactor.netty.ByteBufFlux

	final static Function<Object, ByteBuf> bytebufExtractor = o -> {
		if (o instanceof ByteBuf) {
			return (ByteBuf) o;
		}
		if (o instanceof ByteBufHolder) {
			return ((ByteBufHolder) o).content();
		}
		if (o instanceof byte[]) {
			return Unpooled.wrappedBuffer((byte[]) o);
		}
		throw new IllegalArgumentException("Object " + o + " of type " + o.getClass() + " " + "cannot be converted to ByteBuf");
	};

As this only looks at the content of the LastHttpContent, and ignores the trailerHeaders.

That said, i'm also not really sure how to properly make the trailers available, best i could come up with is an extra mono in the response entity object, as the info would only be available once the full body has been read. But hopefully you can think of something better :)

bmaassenee avatar May 24 '24 07:05 bmaassenee

Currently, neither Spring MVC, Spring WebFlux nor any of our HTTP clients support HTTP trailing headers. This is a requirement for enhancement requests like #24425

bclozel avatar May 24 '24 07:05 bclozel

Reactor Netty server and client support trailing headers. The HttpClient exposes the trailing headers via reactor.netty.http.client.HttpClientResponse#trailerHeaders, for example:

https://github.com/reactor/reactor-netty/blob/89671ea549ce893c74744a602d110cc341a4885e/reactor-netty-http/src/test/java/reactor/netty/http/HttpProtocolsTests.java#L624-L633

violetagg avatar Jul 17 '24 08:07 violetagg

@violetagg does it also make the actual trailer available somehow? As the linked test shows only the headers that announces that a trailer could be present in the response. So right now i've processing it in a semi hacky way:

_httpClient
	.get()
	.uri( _url.get().toString() )
	.responseConnection( this::extract )

and

private Flux<byte[]> extract( HttpClientResponse httpClientResponse, Connection connection )
{
	if( connection instanceof ChannelOperations )
	{
		return ByteBufFlux.fromInbound( ((ChannelOperations<?, ?>) connection ).receiveObject()
				.handle( ( obj, sink ) -> {
					sink.next( obj );
					//check if we finished with a trailer indicating we had a problem
					if( obj instanceof LastHttpContent && ((LastHttpContent) obj).trailingHeaders().contains( "x-een-error" ) )
					{
						sink.error( new ErrorTrailerFoundException(
								( (LastHttpContent) obj ).trailingHeaders().get( "x-een-error" ),
								( (LastHttpContent) obj ).trailingHeaders() )
						);
					}
				} )
		).asByteArray();
	}
	
	throw new IllegalArgumentException( "Unsupported connection type: "+connection.getClass().getName() );
}

bmaassenee avatar Jul 18 '24 07:07 bmaassenee

@bmaassenee The tests checks for actual trainer header.

https://github.com/reactor/reactor-netty/blob/89671ea549ce893c74744a602d110cc341a4885e/reactor-netty-http/src/test/java/reactor/netty/http/HttpProtocolsTests.java#L627

res.responseHeaders() are the response headers res.trailerHeaders() are the trailer headers

violetagg avatar Jul 18 '24 07:07 violetagg

Closing in favour of #33640, as this issue is focusing on WebFlux support: we should support this feature more broadly.

bclozel avatar Oct 03 '24 12:10 bclozel