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

69 天前
 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 ,但是还是感觉挺爽的,记录一下。

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

3700 次点击
所在节点    Java
46 条回复
Martens
68 天前
content-length 有可能变化,那么 content-type 有没有可能也会变?
nekomiao
68 天前
#8 这就是 V2EX 。既不是国内公司维护,主要贡献者里也没国人的影子。这都要无端端拉踩下国内公司还有人点赞。👍👍👍
cookii
68 天前
@Martens 那确实可能也会变。
cedoo22
68 天前
request 构建时修改内容,谁改谁保证功能,不应该框架去保证功能。
securityCoding
68 天前
以前用 spring cloud 编解码 protobuffer 格式遇到这个问题,网关侧写的 content-length 不对
monkeyk
68 天前
这不应该算是 bug ,因为这二者之间不是强绑定关系。
并且框架也是预留了修改的点
ala2008
68 天前
好奇还有没有别的属性也会这样,提供了 set 方法其实可以了
bugmakerxs
68 天前
@Chinsung 如果 content-length 在走 interceptor 之前就设置了,那么 interceptor 中不应该让使用方修改 body 内容。我倾向于是框架的 bug 。
cookii
68 天前
@monkeyk
@bugmakerxs
@cedoo22
@Chinsung
我觉得没必要过于纠结是谁的问题, 但一个框架要让用户知道,我改了 body 的长度,也要修改 content-length 。至少文档上要写。
cookii
68 天前
@ala2008 楼上有说,Content-Type 可能也会改变。不过这个概率更小。
Chinsung
68 天前
@bugmakerxs #28 你把拦截器理解成一个非字节码层面而是 api 层面实现的切面就好了,你对一个东西修改,已经计算好的内容框架是不管你的。
这个只存在人家设计的时候巧不巧,如果他的 content-length 是在最后计算的,那你就会为了他这么智能而感到开心,但是实际上只是运气问题而已
xuanbg
68 天前
这应该算滥用拦截器了吧。。。自己重写一个 execute 方法,在这个方法里面加入 token 不好么
pocketz
68 天前
你既然获得了完全的 request 的控制权,修改 content length 的责任就应该完全在你。
如果只得到了 body ,才有理由相信框架能自动修改 header
wantstark
68 天前
为什么不是以继承的方式呢?
Aresxue
68 天前
属于滥用拦截器了,这个场景写个 Adapter 就解决了,addToken 方法里面肯定还有 json 序列化和反序列化的开销,Adapter 设计合理点还能把这部分开销优化掉。
Vegetable
68 天前
不是 BUG ,合不进去
cctv1005s927
68 天前
恭喜 merge 💐
cookii
68 天前
@cctv1005s927 谢谢😜,我刚看见
imaginistx12
67 天前
恭喜 merge 💐
prosgtsr
67 天前
恭喜🎉

这是一个专为移动设备优化的页面(即为了让你能够在 Google 搜索结果里秒开这个页面),如果你希望参与 V2EX 社区的讨论,你可以继续到 V2EX 上打开本讨论主题的完整版本。

https://www.v2ex.com/t/1069437

V2EX 是创意工作者们的社区,是一个分享自己正在做的有趣事物、交流想法,可以遇见新朋友甚至新机会的地方。

V2EX is a community of developers, designers and creative people.

© 2021 V2EX