记录一次修复 Spring Framework Bug 的经历

[cookii]

Spring RestTemplate 拦截器修改请求体导致的诡异问题

最近在工作中发现了 Spring 的一个"特性"（也许可以叫 Bug ？），反正我已经给 Spring 提了 PR ，等着看能不能合进去。

问题背景

最近在调用第三方 API 时，遇到了一个有意思的场景。整个调用流程大概是这样的：

1. 先调用 /login 接口，发送 username 和 password ，对方服务返回一个 JWT 。
2. 之后的每个请求接口都是标准格式，需要把 JWT 和请求参数放到一个 JSON 中，类似这样：

{
    "token": "JWT-TOKENxxxxxx",
    "data": {
        "key1": "value1",
        "key2": "value2"
    }
}

1. 发送请求，然后拿到响应报文。

解决方案

为了避免在每个接口都重复封装 token ，我想到了用 org.springframework.http.client.ClientHttpRequestInterceptor 来拦截请求，统一修改请求体。

代码大概长这样：

this.restTemplate = new RestTemplateBuilder()
        .requestFactory(() -> new ReactorNettyClientRequestFactory())
        .interceptors((request, body, execution) -> {
            byte[] newBody = addToken(body); // 调用登陆获取 token ，修改入参 body ，添加 token
            return execution.execute(request, newBody);
        })
        .build();

诡异的问题

修改完成后，进入测试阶段，奇怪的事情就发生了：token 能正确获取，body 也修改成功了，但对方的接口一直报 400 ，Invalid JSON 。更奇葩的是，我把 newBody 整个复制出来，用独立的 Main 代码发送请求，居然一次就成功了。

深入源码

不服气的我只能往源码里找原因。从RestTemplate一路 Debug 到org.springframework.http.client.InterceptingClientHttpRequest.InterceptingRequestExecution#execute，发现了这么一段代码：

@Override
public ClientHttpResponse execute(HttpRequest request, byte[] body) throws IOException {
    if (this.iterator.hasNext()) { //这里是在执行 interceptor 链，我的登陆和修改 body 接口就在这里执行
        ClientHttpRequestInterceptor nextInterceptor = this.iterator.next();
        return nextInterceptor.intercept(request, body, this);
    }
    else { // 上面的 interceptor 链执行完后，下面就是真实执行发送请求逻辑
        HttpMethod method = request.getMethod();
        ClientHttpRequest delegate = requestFactory.createRequest(request.getURI(), method);
        request.getHeaders().forEach((key, value) -> delegate.getHeaders().addAll(key, value)); 
        if (body.length > 0) {
            if (delegate instanceof StreamingHttpOutputMessage streamingOutputMessage) {
                streamingOutputMessage.setBody(new StreamingHttpOutputMessage.Body() {
                    @Override
                    public void writeTo(OutputStream outputStream) throws IOException {
                        StreamUtils.copy(body, outputStream);
                    }

                    @Override
                    public boolean repeatable() {
                        return true;
                    }
                });
            }
            else {
                StreamUtils.copy(body, delegate.getBody());
            }
        }
        return delegate.execute();
    }
}

在 Debug 到request.getHeaders().forEach这里时，我突然发现 request 里的Content-Length居然和body.length（被修改后的请求体）不一样。

问题根源

继续往上追溯，在org.springframework.http.client.AbstractBufferingClientHttpRequest中找到了这段代码：

@Override
protected ClientHttpResponse executeInternal(HttpHeaders headers) throws IOException {
    byte[] bytes = this.bufferedOutput.toByteArrayUnsafe();
    if (headers.getContentLength() < 0) {
        headers.setContentLength(bytes.length);
    }
    ClientHttpResponse result = executeInternal(headers, bytes);
    this.bufferedOutput.reset();
    return result;
}

原来Content-Length在执行拦截器之前就已经被设置了。但我们在拦截器里修改了body，导致对方接收到的 JSON 格式总是不对，因为Content-Length和实际的请求体长度不匹配。

解决问题

这时候为了先解决问题，就先在interceptor中重新赋值了Content-Length

this.restTemplate = new RestTemplateBuilder()
        .requestFactory(() -> new ReactorNettyClientRequestFactory())
        .interceptors((request, body, execution) -> {
            byte[] newBody = addToken(body); // 调用登陆获取 token ，修改入参 body ，添加 token
            request.getHeaders().setContentLength(body.length); // 重新设置 Content-Length
            return execution.execute(request, newBody);
        })
        .build();

测试后，问题解决了。

反思和改进

问题虽然解决了，但我琢磨了一下，虽然是我在拦截器中修改了 body ，但这个地方 Spring 应该还是有责任把错误的Content-Length修正的。 第一，Spring 的文档中没有明确写这里应该由谁来负责，是个灰色地带。 第二，我们用RestTemplate谁会自己设置Content-Length啊，不都是框架设置的吗，所以这里不也应该由框架来负责嘛。

思考完，周末找了个时间给 Spring 提了个 PR ，有兴趣的同学可以到这里看看。Update Content-Length when body changed by Interceptor

有一说一，虽然不是第一次提 PR ，但是还是感觉挺爽的，记录一下。

写的挺乱的，技术一般，大佬轻喷。

[chebyshevj]

addToken 应该是自己写的方法吧
如果是的话，个人觉得框架本身不去更新 Content-Length 无可厚非，你既然都在自己写的方法里改了 body ，还是自己把 length 更新更好
想起之前 EasyExcel 里面导出的表头高度问题，在拦截器里面修改表头信息不会更新表头高度，如果你新增了一行那么这行实际上不会导出，因为有有对应的 head()方法自定义表头，这样才会识别出自定义的表头高度
因此如果 addToken 是框架本身提供的方法，那我觉得这会是一个 bug ，反之我持中立态度

[cookii]

@chebyshevj addToken 是自己的方法，但默认的 content-length 也是框架在拦截器之前设置的。这个确实不能算是 bug ，所以我也有说算个灰色地带，框架上确实可以帮你修复这个问题。

[oneisall8955]

并没有觉得是 bug 。发送前，content length 是正确的，自己改了，理应该自己维护

[StevenRCE0]

我个人支持框架将 Content-Length 封装在 interceptor 之下，也正因此不应该像 OP 这样“打补丁”实现。总之 proposal 是好的。

[chihiro2014]

一般消费了请求，得复制一份才行，而且 token 一般不是放在 header 里面吗？

[deplives]

先抛开问题不说 jwt 一般不是在 header 里面吗 放在 body 里面的还真是头一次见

[v2exgo]

@StevenRCE0

那是个 byte[] 对象，content-length 也是 byte[]的长度，大部分程序猿的框架跟语言设计，都是相信程序猿，而不是帮你处处擦屁股

而且 op 这个 更适合在 业务代码调用 rest 封装好的那一层，做一次 AOP 拦截，反射拿到请求的对象，根据对象的 annotation 在对象里面加好 jwt ，

为了偷懒方便 序列化后 每个对象里面都加一个 jwt ， 再一次序列化回去，反正多加了也没事，多一次序列化也就多耗点时，其实还有 jwt token 泄漏的风险，因为这个 client 可能被复用 去调用到不需要 jwt 的地方

[v2exgo]

@deplives 现实就是这样，活久见的设计多了去了，很多时候， 很多事情都是拍脑袋决定的，何况国内公司 升职加薪 业务线完成功能就好了，基本上大家都不看技术好坏，拼的都是关系 跟 办公室政治

[v2exgo]

@StevenRCE0 另外在哪一层做哪一层的事情，你把对象给 restTemplate 它帮你序列化好，把序列化好的内容交给 httpclient 去处理，根据配置或者默认设置 塞好各种 header 值 这是它的职责，因为它给你提供了一层抽象，你无须操心那些细节处理，

但是你到 interceptor 这一层来，你就跟框架到了同一层，你必须明白你在为框架添加什么功能，就跟指针一样，你必须得知道你在使用指针，何时释放它，难道编程语言设计者 没想过指针这种设计 会存在各种 手枪打到自己脚的情况？

[vvhy]

我觉得可以把计算长度和拦截的顺序调换一下？

[wm5d8b]

我记得最早 interceptor 在设计上是用来修改 header 的，后来才被搞出修改 body 的用法。spring 的问题是，没有把包括 length 在内的属性封装成一个 body 对象

[r46mht]

@v2exgo 虽然但是，把 jwt 放到 body 里有什么坏处么...这本来也不是一个技术问题吧

[iyiluo]

这种 PR 估计通不过，运行了这么久的功能，即使是 bug ，也变成了 feature

[sagaxu]

如果是传输编码是 chunked ，setContentLength(body.length)这个 body 是分块后的吗？

[dif]

既然提供了修改长度的方法，那确实很难是 bug ，不过可以发个 issue 询问一下，这一块是这样设计的，还是说是他们的疏忽。

[sagaxu]

@r46mht 把 jwt 放到 body 里有什么坏处么？坏处太大了，网关或中间件本来只解析 header 就能拿到身份信息，放 body 里面就不得不完整解析整个请求了，而 body 有时候比 header 大几个数量级，例如上传文件。如果在 header 里面，完全可以没接收完 http 请求时，就 reset 掉。

[cookii]

@oneisall8955
@StevenRCE0
@v2exgo
@wm5d8b
@iyiluo
各位大佬说得对，修改了 body ，同步修改 Content-Length 是使用者的职责，但 Spring 文档没写这点。

如果 Content-Length 和 Body 不匹配，在不同的 HttpClient 下会有不同的表现：
如果 Content-Length > Body.length ，各种服务端基本上会等客户端发送直到超时。
如果 Body.length > Content-Length, 有些客户端会照常发送出去，至于服务端会怎么处理就要看服务端的逻辑。有些客户端会直接报错。

我觉得如果 Spring 框架不帮你矫正 Content-Length ，那至少也应该报错 or Warning ？

[cookii]

@sagaxu 如果是 chunked ，不能设置 ContentLength 。 不过通常 HTTP 请求很少会用到 chunked 吧，chunked 通常是响应时使用。

[r46mht]

@sagaxu 有道理

[Chinsung]

bug ×
错误的使用 √

[Martens]

content-length 有可能变化，那么 content-type 有没有可能也会变？

[nekomiao]

#8 这就是 V2EX 。既不是国内公司维护，主要贡献者里也没国人的影子。这都要无端端拉踩下国内公司还有人点赞。👍👍👍

[cookii]

@Martens 那确实可能也会变。

[cedoo22]

request 构建时修改内容，谁改谁保证功能，不应该框架去保证功能。

[securityCoding]

以前用 spring cloud 编解码 protobuffer 格式遇到这个问题,网关侧写的 content-length 不对

[monkeyk]

这不应该算是 bug ，因为这二者之间不是强绑定关系。
并且框架也是预留了修改的点

[ala2008]

好奇还有没有别的属性也会这样，提供了 set 方法其实可以了

[bugmakerxs]

@Chinsung 如果 content-length 在走 interceptor 之前就设置了，那么 interceptor 中不应该让使用方修改 body 内容。我倾向于是框架的 bug 。

[cookii]

@monkeyk
@bugmakerxs
@cedoo22
@Chinsung
我觉得没必要过于纠结是谁的问题， 但一个框架要让用户知道，我改了 body 的长度，也要修改 content-length 。至少文档上要写。

[cookii]

@ala2008 楼上有说，Content-Type 可能也会改变。不过这个概率更小。

[Chinsung]

@bugmakerxs #28 你把拦截器理解成一个非字节码层面而是 api 层面实现的切面就好了，你对一个东西修改，已经计算好的内容框架是不管你的。
这个只存在人家设计的时候巧不巧，如果他的 content-length 是在最后计算的，那你就会为了他这么智能而感到开心，但是实际上只是运气问题而已

[xuanbg]

这应该算滥用拦截器了吧。。。自己重写一个 execute 方法，在这个方法里面加入 token 不好么

[pocketz]

你既然获得了完全的 request 的控制权，修改 content length 的责任就应该完全在你。
如果只得到了 body ，才有理由相信框架能自动修改 header

[wantstark]

为什么不是以继承的方式呢？

[Aresxue]

属于滥用拦截器了，这个场景写个 Adapter 就解决了，addToken 方法里面肯定还有 json 序列化和反序列化的开销，Adapter 设计合理点还能把这部分开销优化掉。

[Vegetable]

不是 BUG ，合不进去

[cctv1005s927]

恭喜 merge 💐

[cookii]

@cctv1005s927 谢谢😜，我刚看见

[imaginistx12]

恭喜 merge 💐

[prosgtsr]

恭喜🎉

[billbob]

WebClient 也会有吗? RestTemplate 不是已经停止维护更新了吗?这是一个问题.

[dont39]

恭喜恭喜

[cookii]

@billbob WebClient 是 Reactor 的 client ，我没去看。RestClient 内部用的和 RestTemplate 是一样的东西，同步修复的

[xiaodehenlang]

我理解这个问题涉及到一些设计边界，比如说 body 体我开放了修改内容，那么它的长度理应是可以做到自动化设置的 这个也比较容易做到。但是像 Content-Type 这种 通过程序代码自动识别 准确度和难度都非常大，所以应该由开发者自行维护。

[xiaodehenlang]

OP 的这种精神很好，最近不忙的情况下 有些开源的代码 bug 我也学习提一下 PR

[QWE321ASD]

打脸的是被合并了,充分说明有些人有多僵化