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

SimpleHttpCommand post body长度超过1万,被截 #1253

Closed
zzzzzzzzlllllllll opened this issue Jan 15, 2020 · 14 comments · Fixed by #1255
Closed

SimpleHttpCommand post body长度超过1万,被截 #1253

zzzzzzzzlllllllll opened this issue Jan 15, 2020 · 14 comments · Fixed by #1255
Assignees
Labels
kind/bug Category issues or prs related to bug.

Comments

@zzzzzzzzlllllllll
Copy link

RT

image

这里的buffedReader一次读不完数据。导致body报文被截取。问题能修一下吗。

我们公司在用,现在被迫只能改源码自己打包了。

@jasonjoo2010
Copy link
Collaborator

jasonjoo2010 commented Jan 15, 2020

The reason why it has a capacity with 8kB is that it upgraded from GET implementation. From the RFC the URL is limited to 4kB~8kB.

But it should support larger body and i will give it a shot.

@jasonjoo2010 jasonjoo2010 self-assigned this Jan 15, 2020
@jasonjoo2010 jasonjoo2010 added the kind/bug Category issues or prs related to bug. label Jan 15, 2020
@hasn2360
Copy link

同问,HttpEventTask中针对post请求的较大报文会丢失请求数据

@jasonjoo2010
Copy link
Collaborator

@hasn2360 @zzzzzzzzlllllllll

You can try this pr #1255 to verify.

@zzzzzzzzlllllllll
Copy link
Author

@ hasn2360 @zzzzzzzzlllllllll

您可以尝试使用此PR #1255进行验证。

什么时候能发个1.6.4带上这个pr嘛。我们目前使用的是1.6.0

@jasonjoo2010
Copy link
Collaborator

@ hasn2360 @zzzzzzzzlllllllll
您可以尝试使用此PR #1255进行验证。

什么时候能发个1.6.4带上这个pr嘛。我们目前使用的是1.6.0

Is there any compatible issue when upgrading from 1.6.x to 1.7.x, @sczyh30 ?
Or do some maintaining on 1.6.x? If so maybe something like the encoding issue is worth to be included. If not or they are compatible completely upgrading is the best choice.

从1.6.x升级至1.7.x是否有不兼容?还是我们1.6.x也在维护并发bugfix版?
如果是后者,那么包括encoding的问题,或许也值得做下修补,如果没有不兼容,那还是建议跟上新版本。

@zzzzzzzzlllllllll
Copy link
Author

如果

我司目前生产环境有近千个容器在使用1.6.0。
我们更希望可以优先fixbug,升级版本确实值得提上日程,但不是贸然升级。

@jasonjoo2010
Copy link
Collaborator

如果

我司目前生产环境有近千个容器在使用1.6.0。
我们更希望可以优先fixbug,升级版本确实值得提上日程,但不是贸然升级。

Basically upgrading from 1.6.0 to 1.7.2(for example) maybe not that different to upgrading from 1.6.0 to 1.6.3(or 4). If they are equivalent it won't be a tough decision. Certainly you can also use your own fix directly based on 1.6.0 safely like 1.6.0-fix in your own/private maven repository.

从1.6.0升至1.7.2与从1.6.0升至1.6.3可能没有看起来那么不一样,这个需要Eric来回复确认一下。
当然想要最安全稳妥,可以使用内部的fix版,修改下版本号即可,我们内部有数个这样的版本。

@zzzzzzzzlllllllll
Copy link
Author

那么

哈哈。好的谢谢。我们内部也不太推荐直接修改源码发布内部包。这样升级时还要改造。

我们看了看代码,想到一个有趣的办法。get请求读取第一行时没有长度限制。所以暂时将大请求替换成get了,因为是异步刷新的,对性能要求也不是太高,可以接受。以供后人参考吧。

@jasonjoo2010
Copy link
Collaborator

那么

哈哈。好的谢谢。我们内部也不太推荐直接修改源码发布内部包。这样升级时还要改造。

我们看了看代码,想到一个有趣的办法。get请求读取第一行时没有长度限制。所以暂时将大请求替换成get了,因为是异步刷新的,对性能要求也不是太高,可以接受。以供后人参考吧。

That's a workaround. But not all libraries accept unlimited length of url.

这样能用,但得看库是否有这个限定,根据RFC是有限制的,有的库遵守有的库不遵守。

@sczyh30
Copy link
Member

sczyh30 commented Jan 15, 2020

Basically upgrading from 1.6.0 to 1.7.2(for example) maybe not that different to upgrading from 1.6.0 to 1.6.3(or 4). If they are equivalent it won't be a tough decision. Certainly you can also use your own fix directly based on 1.6.0 safely like 1.6.0-fix in your own/private maven repository.

从1.6.0升至1.7.2与从1.6.0升至1.6.3可能没有看起来那么不一样,这个需要Eric来回复确认一下。
当然想要最安全稳妥,可以使用内部的fix版,修改下版本号即可,我们内部有数个这样的版本。

If you're using 1.6.0, it's recommended to upgrade to the latest 1.7.x version directly. There are very few breaking changes (e.g. ping protocol of cluster flow control) and for most scenarios that's okay.

By the way, we might need to improve the dashboard so that it could keep working with Sentinel < 1.7.1 (#1236) @jasonjoo2010

@sczyh30
Copy link
Member

sczyh30 commented Jan 15, 2020

@ hasn2360 @zzzzzzzzlllllllll
您可以尝试使用此PR #1255进行验证。

什么时候能发个1.6.4带上这个pr嘛。我们目前使用的是1.6.0

This is expected to be released with the next 1.7.x version (1.7.2).

@jasonjoo2010
Copy link
Collaborator

with Sentinel

Yep

Be honest i have not test the newest dashboard to old core library (Be precise we still use an alternated version based on 1.5.x). And it's relevant what the rule/input contains so it will be solved at once if a demo/data/scenario was provided. Do you have one?

@sczyh30
Copy link
Member

sczyh30 commented Jan 15, 2020

with Sentinel

Yep

Be honest i have not test the newest dashboard to old core library (Be precise we still use an alternated version based on 1.5.x). And it's relevant what the rule/input contains so it will be solved at once if a demo/data/scenario was provided. Do you have one?

Refer #1253 (comment)

@sczyh30
Copy link
Member

sczyh30 commented Mar 13, 2020

Resolved in #1255

@sczyh30 sczyh30 closed this as completed Mar 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants