Skip to content

ModifyRequestBodyGatewayFilterFactory creates Content-Type header even when there is no body. #3242

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

Closed
HarryHsuzx opened this issue Jan 26, 2024 · 10 comments

Comments

@HarryHsuzx
Copy link

Describe the bug
Using ModifyRequestBodyGatewayFilterFactory with a rewrite function that returns Mono.empty();, a content-type header is still added to the downstream request which bricks the downstream API contract.
Reproduced on spring cloud gateway 4.1.1 and 3.1.1

Expected behaviour
When the return value is Mono.empty() (cannot be null / Mono.just(null)), the content-type header should not be set. This ensures bodyless requests are not assigned the wrong headers.

Sample
Received request:

2024-01-26T09:30:41.373+08:00 DEBUG 15344 --- [gateway] [ctor-http-nio-3] reactor.netty.http.server.HttpServer     : [86124063, L:/[0:0:0:0:0:0:0:1]:8080 - R:/[0:0:0:0:0:0:0:1]:35221] READ: 162B
         +-------------------------------------------------+
         |  0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f |
+--------+-------------------------------------------------+----------------+
|00000000| 47 45 54 20 2f 61 70 69 2f 64 65 62 75 67 31 2f |GET /api/debug1/|
|00000010| 31 32 33 20 48 54 54 50 2f 31 2e 31 0d 0a 55 73 |123 HTTP/1.1..Us|
|00000020| 65 72 2d 41 67 65 6e 74 3a 20 50 6f 73 74 6d 61 |er-Agent: Postma|
|00000030| 6e 52 75 6e 74 69 6d 65 2f 37 2e 32 38 2e 34 0d |nRuntime/7.28.4.|
|00000040| 0a 41 63 63 65 70 74 3a 20 2a 2f 2a 0d 0a 48 6f |.Accept: */*..Ho|
|00000050| 73 74 3a 20 6c 6f 63 61 6c 68 6f 73 74 3a 38 30 |st: localhost:80|
|00000060| 38 30 0d 0a 41 63 63 65 70 74 2d 45 6e 63 6f 64 |80..Accept-Encod|
|00000070| 69 6e 67 3a 20 67 7a 69 70 2c 20 64 65 66 6c 61 |ing: gzip, defla|
|00000080| 74 65 2c 20 62 72 0d 0a 43 6f 6e 6e 65 63 74 69 |te, br..Connecti|
|00000090| 6f 6e 3a 20 6b 65 65 70 2d 61 6c 69 76 65 0d 0a |on: keep-alive..|
|000000a0| 0d 0a                                           |..              |
+--------+-------------------------------------------------+----------------+

Modified request:

2024-01-26T09:30:41.375+08:00  WARN 15344 --- [gateway] [ctor-http-nio-3] d.SecureHttpPostSignGatewayFilterFactory : MONO.EMPTY returned!
2024-01-26T09:30:41.376+08:00 DEBUG 15344 --- [gateway] [ctor-http-nio-3] reactor.netty.http.client.HttpClient     : [59584b99-4, L:/127.0.0.1:35223 - R:/127.0.0.1:8080] WRITE: 380B
         +-------------------------------------------------+
         |  0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f |
+--------+-------------------------------------------------+----------------+
|00000000| 47 45 54 20 2f 31 2f 31 32 33 20 48 54 54 50 2f |GET /1/123 HTTP/|
|00000010| 31 2e 31 0d 0a 55 73 65 72 2d 41 67 65 6e 74 3a |1.1..User-Agent:|
|00000020| 20 50 6f 73 74 6d 61 6e 52 75 6e 74 69 6d 65 2f | PostmanRuntime/|
|00000030| 37 2e 32 38 2e 34 0d 0a 41 63 63 65 70 74 3a 20 |7.28.4..Accept: |
|00000040| 2a 2f 2a 0d 0a 41 63 63 65 70 74 2d 45 6e 63 6f |*/*..Accept-Enco|
|00000050| 64 69 6e 67 3a 20 67 7a 69 70 2c 20 64 65 66 6c |ding: gzip, defl|
|00000060| 61 74 65 2c 20 62 72 0d 0a 43 6f 6e 74 65 6e 74 |ate, br..Content|
|00000070| 2d 54 79 70 65 3a 20 74 65 78 74 2f 70 6c 61 69 |-Type: text/plai|
|00000080| 6e 3b 63 68 61 72 73 65 74 3d 55 54 46 2d 38 0d |n;charset=UTF-8.|
|00000090| 0a 46 6f 72 77 61 72 64 65 64 3a 20 70 72 6f 74 |.Forwarded: prot|
|000000a0| 6f 3d 68 74 74 70 3b 68 6f 73 74 3d 22 6c 6f 63 |o=http;host="loc|
|000000b0| 61 6c 68 6f 73 74 3a 38 30 38 30 22 3b 66 6f 72 |alhost:8080";for|
|000000c0| 3d 22 5b 30 3a 30 3a 30 3a 30 3a 30 3a 30 3a 30 |="[0:0:0:0:0:0:0|
|000000d0| 3a 31 5d 3a 33 35 32 32 31 22 0d 0a 58 2d 46 6f |:1]:35221"..X-Fo|
|000000e0| 72 77 61 72 64 65 64 2d 46 6f 72 3a 20 30 3a 30 |rwarded-For: 0:0|
|000000f0| 3a 30 3a 30 3a 30 3a 30 3a 30 3a 31 0d 0a 58 2d |:0:0:0:0:0:1..X-|
|00000100| 46 6f 72 77 61 72 64 65 64 2d 50 72 6f 74 6f 3a |Forwarded-Proto:|
|00000110| 20 68 74 74 70 0d 0a 58 2d 46 6f 72 77 61 72 64 | http..X-Forward|
|00000120| 65 64 2d 50 6f 72 74 3a 20 38 30 38 30 0d 0a 58 |ed-Port: 8080..X|
|00000130| 2d 46 6f 72 77 61 72 64 65 64 2d 48 6f 73 74 3a |-Forwarded-Host:|
|00000140| 20 6c 6f 63 61 6c 68 6f 73 74 3a 38 30 38 30 0d | localhost:8080.|
|00000150| 0a 68 6f 73 74 3a 20 31 32 37 2e 30 2e 30 2e 31 |.host: 127.0.0.1|
|00000160| 3a 38 30 38 30 0d 0a 63 6f 6e 74 65 6e 74 2d 6c |:8080..content-l|
|00000170| 65 6e 67 74 68 3a 20 30 0d 0a 0d 0a             |ength: 0....    |
+--------+-------------------------------------------------+----------------+

Note there is a Content-Type: text/plainlcharset=UTF-8 header in the downstream request.

RewriteFunction

@Override
    public Publisher<String> apply(ServerWebExchange exchange, String body) {
        // Escape if not POST
        if (exchange.getRequest().getMethod() != HttpMethod.POST) {
            logger.warn("MONO.EMPTY returned!");
            return Mono.empty();
        }

        //.....
    }

I've made a mre with the version 4.1.1, but this behaviour also exists in the 3.x versions. The repo is as below:
https://github.com/HarryHsuzx/scgRewriteFunctionContentTypeMRE/blob/master/src/main/resources/application.yml

@spencergibb
Copy link
Member

I suppose we could condition the setting of a different content type only if content length is greater than 0 here

if (config.getContentType() != null) {
headers.set(HttpHeaders.CONTENT_TYPE, config.getContentType());
}

@vasantteja
Copy link

Hi can I work on this?

@spencergibb
Copy link
Member

It's been assigned to you

@NadChel
Copy link
Contributor

NadChel commented Apr 10, 2024

I suppose we could condition the setting of a different content type only if content length is greater than 0 here

@spencergibb We don't know anything about the actual content length before the body Mono is subscribed to (which happens beyond the scope of the filter() method)

I reproduced it, wrote a quick fix, but then realized org.springframework.http.codec.EncoderHttpMessageWriter sets a default content type anyway (which is in my case text/plain;charset=UTF-8)

	@Nullable
	private MediaType updateContentType(ReactiveHttpOutputMessage message, @Nullable MediaType mediaType) {
		MediaType result = message.getHeaders().getContentType();
		if (result != null) {
			return result;
		}
		MediaType fallback = this.defaultMediaType;
		result = (useFallback(mediaType, fallback) ? fallback : mediaType);
		if (result != null) {
			result = addDefaultCharset(result, fallback);
			message.getHeaders().setContentType(result);
		}
		return result;
	}

I figure it means it's blocked for now

I created a separate issue for the blocker as well as a PR that fixes it. I hope it will be merged soon

@NadChel

This comment has been minimized.

@NadChel
Copy link
Contributor

NadChel commented Apr 26, 2024

@spencergibb my PR was merged a few days ago. As soon as we're past an upcoming 6.2M2 milestone, I'm all good to submit a fix for this issue (as long as @vasantteja doesn't mind as an assignee). Feel free to tag me when it's the time

@chrisholly-skyuk
Copy link

chrisholly-skyuk commented Aug 8, 2024

@NadChel any update on fixing this issue?

@NadChel
Copy link
Contributor

NadChel commented Aug 10, 2024

@spencergibb was the Spring dependency updated yet (the one containing EncoderHttpMessageWriter)?

@mehrdadbozorgmehr
Copy link

Can I work on this issue because it's still opened?

@HarryHsuzx
Copy link
Author

@mehrdadbozorgmehr

Can I work on this issue because it's still opened?

I think it's been fixed since 00b862c with spring framework 6.2. I upgraded the dependency on the demo project from 4.1.1 to 4.2.0 and the content-type header no longer exists in the forwarded request. see branch fixed-version

@NadChel @spencergibb
I think this issue can be closed

@vasantteja vasantteja removed their assignment Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants