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

HttpServerOperations overrides content-length to 0 on HTTP HEAD #25908

Closed
violetagg opened this issue Oct 13, 2020 · 2 comments
Closed

HttpServerOperations overrides content-length to 0 on HTTP HEAD #25908

violetagg opened this issue Oct 13, 2020 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@violetagg
Copy link
Member

violetagg commented Oct 13, 2020

Affects: 5.2.9.RELEASE


According to the specification RFC https://tools.ietf.org/html/rfc7231#section-4.3.2

The server SHOULD send the same
   header fields in response to a HEAD request as it would have sent if
   the request had been a GET, except that the payload header fields
   (Section 3.3) MAY be omitted.

This is the endpoint

private final DataBufferFactory dataBufferFactory = new DefaultDataBufferFactory();

@GetMapping("/headers")
public ResponseEntity<Flux<DataBuffer>> testHeadersGET() {
	return ResponseEntity
			.ok()
			.body(Flux.defer(() -> {
				byte[] bytes = "variableLengthString".getBytes();
					 
				return Flux.just(dataBufferFactory.wrap(bytes));
			}));
}

With GET request these are the request/response headers

GET /headers HTTP/1.1
Host: localhost:8080
Accept: */*
HTTP/1.1 200 OK
transfer-encoding: chunked
Content-Type: text/event-stream

With HEAD request these are the request/response headers

GET /headers HTTP/1.1
Host: localhost:8080
Accept: */*
HTTP/1.1 200 OK
Content-Type: text/event-stream
Content-Length: 20

In the first case with GET, transfer-encoding is used, while in the case with HEAD it is Content-Length: 20. Why is that?

Also additional question why Spring Framework decides to return Content-Type: text/event-stream as it is not specified that this is SSE.

This is related to reactor/reactor-netty#1333

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Oct 13, 2020

The first issue is that both DataBufferEncoder and ServerSentHttpMessageWriter return true from canWrite for Flux<DataBuffer> but the choice to write SSE should be made only if there is something more explicit such as a media type hint (via Accept header or a produces condition) or ServerSentEvent as the return type. By comparison Spring MVC only writes SSE in such conditions. Based on this, "text/event-stream" is the chosen content type but DataBufferEncoder is still first in the order and used to write.

When this is corrected, "application/octet-stream" is the chosen content type as a fallback. However there is a second issue. For HTTP HEAD in HttpHeadResponseDecorator we aggregate a Flux and set the content-length. Generally for HTTP GET we set the content-length only for a Mono in EncoderHttpMessageWriter and most other HttpMessageWriter don't set it either. So we are a little too aggressive in setting a content-length for a Flux return value which means HEAD and GET won't have the exact same headers but having this on HEAD is also helpful in a way.

A third issue is that DataBufferEncoder does not implement HttpMessageEncoder and therefore does not have any knowledge of streaming media types. That means even if the media type was "text/event-stream", EncoderHttpMessageWriter would still write as a regular response via writeWith instead of via writeAndFlushWith. This could come up in a scenario with a proxy that is simply passing raw data from an already fully formatted stream.

@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 13, 2020
@rstoyanchev rstoyanchev changed the title Different response headers are returned when HEAD is used compared to GET (one and the same endpoint) ServerSentEventHttpMessageWriter should not write Flux<DataBuffer> without a content type hint Oct 13, 2020
@rstoyanchev rstoyanchev added this to the 5.2.10 milestone Oct 13, 2020
@rstoyanchev
Copy link
Contributor

I've updated HttpHeadResponseDecorator in 5.2.x to avoid adding Content-Length if there is an existing Content-Length and Transfer-Encoding header.

I've extended this further in 5.3 so that Content-Length is only calculated for a Mono since normally for a GET HttpMessageWriter implementations typically do not set a Content-Length for a Flux.

@rstoyanchev rstoyanchev changed the title ServerSentEventHttpMessageWriter should not write Flux<DataBuffer> without a content type hint HttpSeverOperations overrides content-length to 0 on HTTP HEAD Oct 19, 2020
@jhoeller jhoeller changed the title HttpSeverOperations overrides content-length to 0 on HTTP HEAD HttpServerOperations overrides content-length to 0 on HTTP HEAD Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants