V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
cookii
V2EX  ›  Java

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

  •  
  •   cookii · 104 天前 · 3786 次点击
    这是一个创建于 104 天前的主题,其中的信息可能已经有所发展或是发生改变。

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

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

    46 条回复    2024-09-14 14:42:23 +08:00
    chebyshevj
        1
    chebyshevj  
       104 天前
    addToken 应该是自己写的方法吧
    如果是的话,个人觉得框架本身不去更新 Content-Length 无可厚非,你既然都在自己写的方法里改了 body ,还是自己把 length 更新更好
    想起之前 EasyExcel 里面导出的表头高度问题,在拦截器里面修改表头信息不会更新表头高度,如果你新增了一行那么这行实际上不会导出,因为有有对应的 head()方法自定义表头,这样才会识别出自定义的表头高度
    因此如果 addToken 是框架本身提供的方法,那我觉得这会是一个 bug ,反之我持中立态度
    cookii
        2
    cookii  
    OP
       104 天前 via Android
    @chebyshevj addToken 是自己的方法,但默认的 content-length 也是框架在拦截器之前设置的。这个确实不能算是 bug ,所以我也有说算个灰色地带,框架上确实可以帮你修复这个问题。
    oneisall8955
        3
    oneisall8955  
       104 天前   ❤️ 2
    并没有觉得是 bug 。发送前,content length 是正确的,自己改了,理应该自己维护
    StevenRCE0
        4
    StevenRCE0  
       104 天前
    我个人支持框架将 Content-Length 封装在 interceptor 之下,也正因此不应该像 OP 这样“打补丁”实现。总之 proposal 是好的。
    chihiro2014
        5
    chihiro2014  
       104 天前
    一般消费了请求,得复制一份才行,而且 token 一般不是放在 header 里面吗?
    deplives
        6
    deplives  
       104 天前 via iPhone
    先抛开问题不说 jwt 一般不是在 header 里面吗 放在 body 里面的还真是头一次见
    v2exgo
        7
    v2exgo  
       104 天前
    @StevenRCE0

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

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

    为了偷懒方便 序列化后 每个对象里面都加一个 jwt , 再一次序列化回去,反正多加了也没事,多一次序列化也就多耗点时,其实还有 jwt token 泄漏的风险,因为这个 client 可能被复用 去调用到不需要 jwt 的地方
    v2exgo
        8
    v2exgo  
       104 天前   ❤️ 1
    @deplives 现实就是这样,活久见的设计多了去了,很多时候, 很多事情都是拍脑袋决定的,何况国内公司 升职加薪 业务线完成功能就好了,基本上大家都不看技术好坏,拼的都是关系 跟 办公室政治
    v2exgo
        9
    v2exgo  
       104 天前
    @StevenRCE0 另外在哪一层做哪一层的事情,你把对象给 restTemplate 它帮你序列化好,把序列化好的内容交给 httpclient 去处理,根据配置或者默认设置 塞好各种 header 值 这是它的职责,因为它给你提供了一层抽象,你无须操心那些细节处理,

    但是你到 interceptor 这一层来,你就跟框架到了同一层,你必须明白你在为框架添加什么功能,就跟指针一样,你必须得知道你在使用指针,何时释放它,难道编程语言设计者 没想过指针这种设计 会存在各种 手枪打到自己脚的情况?
    vvhy
        10
    vvhy  
       103 天前 via Android   ❤️ 1
    我觉得可以把计算长度和拦截的顺序调换一下?
    wm5d8b
        11
    wm5d8b  
       103 天前 via Android
    我记得最早 interceptor 在设计上是用来修改 header 的,后来才被搞出修改 body 的用法。spring 的问题是,没有把包括 length 在内的属性封装成一个 body 对象
    r46mht
        12
    r46mht  
       103 天前
    @v2exgo 虽然但是,把 jwt 放到 body 里有什么坏处么...这本来也不是一个技术问题吧
    iyiluo
        13
    iyiluo  
       103 天前
    这种 PR 估计通不过,运行了这么久的功能,即使是 bug ,也变成了 feature
    sagaxu
        14
    sagaxu  
       103 天前
    如果是传输编码是 chunked ,setContentLength(body.length)这个 body 是分块后的吗?
    dif
        15
    dif  
       103 天前
    既然提供了修改长度的方法,那确实很难是 bug ,不过可以发个 issue 询问一下,这一块是这样设计的,还是说是他们的疏忽。
    sagaxu
        16
    sagaxu  
       103 天前
    @r46mht 把 jwt 放到 body 里有什么坏处么?坏处太大了,网关或中间件本来只解析 header 就能拿到身份信息,放 body 里面就不得不完整解析整个请求了,而 body 有时候比 header 大几个数量级,例如上传文件。如果在 header 里面,完全可以没接收完 http 请求时,就 reset 掉。
    cookii
        17
    cookii  
    OP
       103 天前
    @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
        18
    cookii  
    OP
       103 天前
    @sagaxu 如果是 chunked ,不能设置 ContentLength 。 不过通常 HTTP 请求很少会用到 chunked 吧,chunked 通常是响应时使用。
    r46mht
        19
    r46mht  
       103 天前
    @sagaxu 有道理
    Chinsung
        20
    Chinsung  
       103 天前
    bug ×
    错误的使用 √
    Martens
        21
    Martens  
       103 天前
    content-length 有可能变化,那么 content-type 有没有可能也会变?
    nekomiao
        22
    nekomiao  
       103 天前   ❤️ 5
    #8 这就是 V2EX 。既不是国内公司维护,主要贡献者里也没国人的影子。这都要无端端拉踩下国内公司还有人点赞。👍👍👍
    cookii
        23
    cookii  
    OP
       103 天前
    @Martens 那确实可能也会变。
    cedoo22
        24
    cedoo22  
       103 天前
    request 构建时修改内容,谁改谁保证功能,不应该框架去保证功能。
    securityCoding
        25
    securityCoding  
       103 天前
    以前用 spring cloud 编解码 protobuffer 格式遇到这个问题,网关侧写的 content-length 不对
    monkeyk
        26
    monkeyk  
       103 天前
    这不应该算是 bug ,因为这二者之间不是强绑定关系。
    并且框架也是预留了修改的点
    ala2008
        27
    ala2008  
       103 天前
    好奇还有没有别的属性也会这样,提供了 set 方法其实可以了
    bugmakerxs
        28
    bugmakerxs  
       103 天前
    @Chinsung 如果 content-length 在走 interceptor 之前就设置了,那么 interceptor 中不应该让使用方修改 body 内容。我倾向于是框架的 bug 。
    cookii
        29
    cookii  
    OP
       103 天前
    @monkeyk
    @bugmakerxs
    @cedoo22
    @Chinsung
    我觉得没必要过于纠结是谁的问题, 但一个框架要让用户知道,我改了 body 的长度,也要修改 content-length 。至少文档上要写。
    cookii
        30
    cookii  
    OP
       103 天前
    @ala2008 楼上有说,Content-Type 可能也会改变。不过这个概率更小。
    Chinsung
        31
    Chinsung  
       103 天前
    @bugmakerxs #28 你把拦截器理解成一个非字节码层面而是 api 层面实现的切面就好了,你对一个东西修改,已经计算好的内容框架是不管你的。
    这个只存在人家设计的时候巧不巧,如果他的 content-length 是在最后计算的,那你就会为了他这么智能而感到开心,但是实际上只是运气问题而已
    xuanbg
        32
    xuanbg  
       103 天前
    这应该算滥用拦截器了吧。。。自己重写一个 execute 方法,在这个方法里面加入 token 不好么
    pocketz
        33
    pocketz  
       103 天前   ❤️ 1
    你既然获得了完全的 request 的控制权,修改 content length 的责任就应该完全在你。
    如果只得到了 body ,才有理由相信框架能自动修改 header
    wantstark
        34
    wantstark  
       103 天前
    为什么不是以继承的方式呢?
    Aresxue
        35
    Aresxue  
       103 天前
    属于滥用拦截器了,这个场景写个 Adapter 就解决了,addToken 方法里面肯定还有 json 序列化和反序列化的开销,Adapter 设计合理点还能把这部分开销优化掉。
    Vegetable
        36
    Vegetable  
       103 天前
    不是 BUG ,合不进去
    cctv1005s927
        37
    cctv1005s927  
       103 天前
    恭喜 merge 💐
    cookii
        38
    cookii  
    OP
       103 天前 via Android
    @cctv1005s927 谢谢😜,我刚看见
    imaginistx12
        39
    imaginistx12  
       103 天前
    恭喜 merge 💐
    prosgtsr
        40
    prosgtsr  
       102 天前 via iPhone
    恭喜🎉
    billbob
        41
    billbob  
       102 天前
    WebClient 也会有吗? RestTemplate 不是已经停止维护更新了吗?这是一个问题.
    dont39
        42
    dont39  
       102 天前
    恭喜恭喜
    cookii
        43
    cookii  
    OP
       102 天前
    @billbob WebClient 是 Reactor 的 client ,我没去看。RestClient 内部用的和 RestTemplate 是一样的东西,同步修复的
    xiaodehenlang
        44
    xiaodehenlang  
       101 天前
    我理解这个问题涉及到一些设计边界,比如说 body 体我开放了修改内容,那么它的长度理应是可以做到自动化设置的 这个也比较容易做到。但是像 Content-Type 这种 通过程序代码自动识别 准确度和难度都非常大,所以应该由开发者自行维护。
    xiaodehenlang
        45
    xiaodehenlang  
       101 天前
    OP 的这种精神很好,最近不忙的情况下 有些开源的代码 bug 我也学习提一下 PR
    QWE321ASD
        46
    QWE321ASD  
       91 天前
    打脸的是被合并了,充分说明有些人有多僵化
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   实用小工具   ·   991 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 22ms · UTC 23:58 · PVG 07:58 · LAX 15:58 · JFK 18:58
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.