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

HTTP codec auto may incorrectly select http/2 #8229

Closed
zuercher opened this issue Sep 12, 2019 · 1 comment · Fixed by #8232
Closed

HTTP codec auto may incorrectly select http/2 #8229

zuercher opened this issue Sep 12, 2019 · 1 comment · Fixed by #8232
Assignees
Labels
Milestone

Comments

@zuercher
Copy link
Member

When the HTTP connection manager codec is set to type AUTO, it searches the first buffer on the connection for the string PRI * HTTP/2. If that string occurs anywhere in the buffer, Envoy attempts to negotiate HTTP/2 and fails. If the string appears in an HTTP header or request body it can trigger HTTP/2 incorrectly.

Repro steps:

Start envoy with the config below and issue a request containing the string:

$ curl -X POST --data "something something PRI * HTTP/2" localhost:8000/foo
curl: (52) Empty reply from server

Log reports:

[2019-09-12 14:59:50.891][3032142][debug][http] [external/envoy/source/common/http/conn_manager_impl.cc:284] [C4] dispatch error: Received bad client magic byte string

Config:

node:
  id: "plain"
  cluster: "plain"
static_resources:
  listeners:
    - name: plain
      address:
        socket_address:
          protocol: TCP
          address: 0.0.0.0
          port_value: 8000
      filter_chains:
        - filters:
            - name: envoy.http_connection_manager
              config:
                idle_timeout: 1s
                codec_type: auto
                stat_prefix: ingress_http
                route_config:
                  name: plain
                  virtual_hosts:
                    - name: plain
                      domains:
                        - "*"
                      routes:
                        - match:
                            prefix: "/"
                          route:
                            cluster: plain
                http_filters:
                  - name: envoy.router
                    typed_config: {}

  clusters:
    - name: plain
      connect_timeout: 0.25s
      type: STATIC
      lb_policy: round_robin
      hosts:
        - socket_address:
            address: "127.0.0.1"
            port_value: 8080
admin:
  access_log_path: "/dev/null"
  address:
    socket_address:
      address: 0.0.0.0
      port_value: 9000
@zuercher zuercher self-assigned this Sep 12, 2019
@zuercher
Copy link
Member Author

@zuercher zuercher added the bug label Sep 12, 2019
zuercher added a commit to zuercher/envoy that referenced this issue Sep 13, 2019
Risk Level: low
Testing: added test case
Docs Changes: n/a
Release Notes: n/a
Fixes: envoyproxy#8229

Signed-off-by: Stephan Zuercher <[email protected]>
@mattklein123 mattklein123 added this to the 1.12.0 milestone Sep 15, 2019
zuercher added a commit that referenced this issue Sep 18, 2019
http: only accept HTTP client magic at the start of buffer

Risk Level: low
Testing: added test case
Docs Changes: n/a
Release Notes: updated
Fixes: #8229

Signed-off-by: Stephan Zuercher <[email protected]>
danzh2010 pushed a commit to danzh2010/envoy that referenced this issue Sep 24, 2019
…y#8232)

http: only accept HTTP client magic at the start of buffer

Risk Level: low
Testing: added test case
Docs Changes: n/a
Release Notes: updated
Fixes: envoyproxy#8229

Signed-off-by: Stephan Zuercher <[email protected]>
danzh2010 pushed a commit to danzh2010/envoy that referenced this issue Oct 4, 2019
…y#8232)

http: only accept HTTP client magic at the start of buffer

Risk Level: low
Testing: added test case
Docs Changes: n/a
Release Notes: updated
Fixes: envoyproxy#8229

Signed-off-by: Stephan Zuercher <[email protected]>
danzh2010 pushed a commit to danzh2010/envoy that referenced this issue Oct 4, 2019
…y#8232)

http: only accept HTTP client magic at the start of buffer

Risk Level: low
Testing: added test case
Docs Changes: n/a
Release Notes: updated
Fixes: envoyproxy#8229

Signed-off-by: Stephan Zuercher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants