Allenxuxu / gev

🚀Gev is a lightweight, fast non-blocking TCP network library / websocket server based on Reactor mode. Support custom protocols to quickly and easily build high-performance servers.
MIT License
1.73k stars 194 forks source link

bugfix: fix(#85) #86

Closed cs-charles closed 3 years ago

cs-charles commented 3 years ago

bugfix: fix(#85)

Allenxuxu commented 3 years ago

image 感觉是不是在这块后面加上 err 的处理比较合理,之前漏掉了 err 的判断。

cs-charles commented 3 years ago

确实,应该在for循环里面加判断会跟加合理

cs-charles commented 3 years ago

如果是在for循环开头加上err判断是否可以

for i := 1; i < len(lines); i++ {
            if len(lines[i]) == 0 || err != nil {
                // Blank line, no more lines to read.
                break
            }
        ....
}
Allenxuxu commented 3 years ago

如果是在for循环开头加上err判断是否可以

for i := 1; i < len(lines); i++ {
            if len(lines[i]) == 0 || err != nil {
              // Blank line, no more lines to read.
              break
            }
        ....
}

不光是这里吧,这个函数上面也有 err 漏判的。 这里 for 循环里的 err ,感觉可以 err = xx 然后break,在循环后面统一 if err != nil 处理。

cs-charles commented 3 years ago

辛苦大佬有空修复一下这个bug

Allenxuxu commented 3 years ago

辛苦大佬有空修复一下这个bug

你不是都写了吗,直接修改下,推上来吧

cs-charles commented 3 years ago

辛苦大佬有空修复一下这个bug

你不是都写了吗,直接修改下,推上来吧

辛苦review一下

Allenxuxu commented 3 years ago

咦,怎么是向 plugin 分支发起的pr

cs-charles commented 3 years ago

咦,怎么是向 plugin 分支发起的pr

我不太确定你们是否是plugin分支开发,然后提交到master,需要更改么。另外想请教你一个问题,我们公司内部的云上的LB会把http请求头的Connection: Upgrade改成Connection: upgrade导致请求升级失败。框架能否修改支持不区分大小写来校验呢

Allenxuxu commented 3 years ago

暂时不用改吧,之后我来合到master吧。

大小写的问题,之前有mr修过了,不过还没发tag 似乎。 https://github.com/Allenxuxu/gev/pull/82

Allenxuxu commented 3 years ago

https://github.com/Allenxuxu/gev/issues/38 可以在这里面补充下公司信息吗😄

cs-charles commented 3 years ago

38

可以在这里面补充下公司信息吗😄

补充了,我们近期准备做压测😂,上述两个问题辛苦尽快打一个tag发布。

cs-charles commented 3 years ago

38

可以在这里面补充下公司信息吗😄

抱歉,我刚删除了公司信息,我得先内部问一下此举是否合规😅。如果合规后续我会补上。

Allenxuxu commented 3 years ago

38

可以在这里面补充下公司信息吗😄

补充了,我们近期准备做压测😂,上述两个问题辛苦尽快打一个tag发布。

好,今晚我就发一个

Allenxuxu commented 3 years ago

尴尬了,plugin 这个分支暂时还不能合并(这个分支的功能还不太适合合并),你能重新提个 pr 到 master 吗 @cs-charles 。 合并完我给你打个 tag。

cs-charles commented 3 years ago

尴尬了,plugin 这个分支暂时还不能合并(这个分支的功能还不太适合合并),你能重新提个 pr 到 master 吗 @cs-charles 。 合并完我给你打个 tag。

可以的

Allenxuxu commented 3 years ago

@cs-charles v0.2.3 已发布

Allenxuxu commented 3 years ago

38

可以在这里面补充下公司信息吗😄

抱歉,我刚删除了公司信息,我得先内部问一下此举是否合规😅。如果合规后续我会补上。

怎么样,应该合规的吧,可以补上吗😂

cs-charles commented 3 years ago

38

可以在这里面补充下公司信息吗😄

抱歉,我刚删除了公司信息,我得先内部问一下此举是否合规😅。如果合规后续我会补上。

怎么样,应该合规的吧,可以补上吗😂

已加上🤝🤝