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

fix:http转grpc响应头设置失败 #113

Merged
merged 1 commit into from Jun 6, 2023
Merged

Conversation

baker-yuan
Copy link
Contributor

No description provided.

@Dot-Liu
Copy link
Collaborator

Dot-Liu commented Jun 5, 2023

稍等,我们确定一下

@baker-yuan
Copy link
Contributor Author

http方法发送请求走的是
node/http-context/context.go#SendTo,这里会刷新响应头,但是gRPC转换没走这个方法,需要手动刷新下,http这里的刷新是不是也可以去掉了统一放在node/http-context/header.go#ResponseHeader#refresh这里刷新响应头

@Dot-Liu
Copy link
Collaborator

Dot-Liu commented Jun 6, 2023

经过确定,只有http需要,http转gRPC是不需要这么写的
原因如下:
1、http的response是直接让fasthttp重置了,在转发前设置的响应头会被置空,因此需要先读出来,再请求后重新设置
2、grpc的complete方法,对原response设置响应头,所以不会有这个问题

@baker-yuan
Copy link
Contributor Author

baker-yuan commented Jun 6, 2023

经过确定,只有http需要,http转gRPC是不需要这么写的 原因如下: 1、http的response是直接让fasthttp重置了,在转发前设置的响应头会被置空,因此需要先读出来,再请求后重新设置 2、grpc的complete方法,对原response设置响应头,所以不会有这个问题

好的,我在看看,可以实际测试下,我测试就是不行,我也是调试代码发现的,返回的Content-Type是text/plain

curl -X POST  'http://127.0.0.1:9400/api/setting/plugin' \
-H 'Content-Type:application/json' \
-d '{
    "plugins":[{
        "id":"eolinker.com:apinto:http_to_grpc",
        "name":"http_to_grpc",
        "status":"enable"
    }]
}'
curl -X POST  'http://127.0.0.1:9400/api/service' -H 'Content-Type:application/json' -d '{
    "name":"grpc_api",
    "driver":"http",
    "description":"grpc服务",
    "timeout":2000,
    "retry":3,
    "scheme":"http",
    "nodes":[
        "127.0.0.1:9001"
    ],
    "balance":"round-robin"
}' 
curl -X POST  'http://127.0.0.1:9400/api/router' -H 'Content-Type:application/json' -d '{
    "name":"grpc_router",
    "driver": "http",
    "description":"grpc路由",
    "listen":80,
    "method":[
        "GET",
        "POST"
    ],
    "host":[
    ],
    "location":"/Service.Hello/Hello",
    "rules":[
    ],
    "service":"grpc_api@service",
    "plugins":{
        "http_to_grpc":{
            "disable":false,
            "config":{
                "service":"Service.Hello",
                "method":"Hello",
                "authority":"",
                "format":"json",
                "headers":{
                },
                "reflect":true
            }
        }
    },
    "retry":3,
    "time_out":2000
}' 
curl --location --request POST 'http://127.0.0.1:80/Service.Hello/Hello' \
--header 'Content-Type: application/json' \
--header 'Cookie: uid=1' \
--data-raw '{
    "name": "baker"
}'

@Dot-Liu
Copy link
Collaborator

Dot-Liu commented Jun 6, 2023

了解,我们这边复测看看

@Dot-Liu
Copy link
Collaborator

Dot-Liu commented Jun 6, 2023

您好,根据您的反馈,我们已经复现了,不过修改的地方不是Finish
您可以重新提交更新,我们再进行合并,需要改动的代码如下
image
image

@baker-yuan
Copy link
Contributor Author

您好,根据您的反馈,我们已经复现了,不过修改的地方不是Finish 您可以重新提交更新,我们再进行合并,需要改动的代码如下 image image

好的好的,马上提交 [狗头]

@baker-yuan baker-yuan force-pushed the patch-1 branch 3 times, most recently from 8146e87 to 8bde310 Compare June 6, 2023 12:15
@Dot-Liu
Copy link
Collaborator

Dot-Liu commented Jun 6, 2023

node/http-context/header.go
这个文件少了
r.header.Set(key, value)操作

@Dot-Liu Dot-Liu merged commit 71d19a5 into eolinker:main Jun 6, 2023
@baker-yuan baker-yuan mentioned this pull request Jun 15, 2023
@baker-yuan
Copy link
Contributor Author

有个疑问,上次改这个 grpc header失效的问题
1、AddHeader和DelHeader是不是也要跟着该一下。
2、afterProxy的作用是干嘛的,有点没理解到

@hmzzrcs
Copy link
Collaborator

hmzzrcs commented Jun 19, 2023

有个疑问,上次改这个 grpc header失效的问题 1、AddHeader和DelHeader是不是也要跟着该一下。 2、afterProxy的作用是干嘛的,有点没理解到

1 确实要跟着改,上次大意了
问题2, 这个问题和这个responseHeader的设计有关,http 转发我们用了一个快捷方式来复制上游返回的response,就会导致在http协议下,转发完成后,在转发前对响应的header设置被覆盖,针对这种情况,我们将转发前的header操作添加到actions中,在转发完成后执行refresh进行重放,之前的版本为了些许性能,在转发前只记录action而不对header执行操作。 而在转换成其他协议后,由于没有触发refresh的操作,导致action中的记录没有重放
上次修改由于疏忽,漏了 AddHeader和DelHeader的操作,我们会尽快把修复的代码发布

@hmzzrcs
Copy link
Collaborator

hmzzrcs commented Jun 19, 2023

修改和SetHeader一样,您可用修改后提交一个pull,成为我们的贡献者

@baker-yuan
Copy link
Contributor Author

baker-yuan commented Jun 20, 2023

修改和SetHeader一样,您可用修改后提交一个pull,成为我们的贡献者

好的好的 https://github.com/eolinker/apinto/pull/117/files

@baker-yuan baker-yuan deleted the patch-1 branch June 21, 2023 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants