etcd
etcd copied to clipboard
[3.4] etcdserver: fix broken APIv2 with long URI
APIv2 doesn't support operations with URI length > 4k. This could be relatively easy achieved by CAS operations with value size about 2k.
This is not documented issue caused by using cmux to serve HTTP1
and HTTP2 protocols on the same port (2379 by default).
cmux has hardcoded max request size for HTTP1 matcher
and returns false
if size is greater.
HTTP1Fast matcher from cmux validates requests only against known
HTTP methods (GET, POST, PUT, DELETE, OPTIONS,..)
and doesn't have such limit.
Fixes #14023
FYI V2 API was deprecated for couple of releases and is already removed on main branch, the only remaining things are for serving /metrics
and /version
endpoints.
If you would like to continue with a PR, for example to backport it to old releases, please add tests.
@serathius I've changed destination branch of the PR to release-3.4 (for version which we actually use on the production) and added a test. Pls advise me if I did it in the wrong way.
@Hexta Do you see any real issue in your test or just by reviewing the source code?
Note that the cmux.HTTP1() is just reading the first line in the HTTP header. In the following example, it's just reading the first line GET /examples/my-awesome-test/ HTTP/1.1
. Also see matchers.go#L94
GET /examples/my-awesome-test/ HTTP/1.1
Host: code.tutsplus.com
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive
Cookie: PHPSESSID=r2t5uvjq435r4q7ib3vtdjq120
Pragma: no-cache
Cache-Control: no-cache
@ahrtr Try to do the same test with URI longer than 4096. Yes, I've the real issue on the production.
@ahrtr Try to do the same test with URI longer than 4096. Yes, I've the real issue on the production.
Got it. So you run into this issue when there is a long long prevValue
?
Just as @serathius pointed out, v2 API is deprecated, and the v2 server & etcdctl v2 commands have already been removed in main branch (3.6). Can you migrate to the v3 API? You have to perform the migration someday, it's just a matter of time.
+1 to migration to v3.
-
v3.4 is rather EOL. We cherry pick there security patches, but not semantical improvements. I'm a little bit concerned that
Http1Fast
might bring unexpected semantical differences. To be on the safe side, it should be flag gated... but than it becomes substantial change in v3.4. -
If we decided to merge - we would need to update the changelog file as well for v3.4.
That's really ironic. We started migration to v3 and replaced etcd proxy with etcd gateway to provide APIv2 and v3 to our services simultaneously. And we faced with broken CAS operations with values >4K which is much lower than documented etcd limits.
I'm sorry, I don't understand. Do you mean that you application was working correctly over V2 API and not hitting the limites, and the problem started when the traffic becomes forwarded through the gateway ?
@ptabor Exactly.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.