[ResponseOps] Reports are timing out and failing with an invalid header error

[doakalexi]

Resolves #225915 (comment)

Summary

Kibana is using HTTP/2 by default, and it's including pseudo-headers (headers prefixed by a colon :) in the reporting requests to ES. The psuedo-headers are causing reports to fail with this error, Failed to complete a request using headers: Protocol error (Fetch.continueRequest): Invalid header, when running Kibana locally with --ssl. This PR updates stripUnsafeHeaders() to remove psuedo-headers.

Checklist

• Unit or functional tests were updated or added to match the most common scenarios

To verify

1. Run Kibana and ES with --ssl
2. Generate a PDF report (with and without printing) and a PNG report and verify that the report is generated successfully.
3. Repeat step 2 but run Kibana without --ssl

[elasticmachine]

Pinging @elastic/response-ops (Team:ResponseOps)

[pmuellr]

We will also need to backport to whatever open 8.x releases we have - I think doing 8.17 probably makes sense as well, unless it's a difficult merge conflict to fix (it may never ship another patch release, but ... I doubt it :-).

The ability to use http2 was added in 8.15.0, so it's possible for a customer using 8.x to run into this same problem, if they specifically set the config server.protocol: http2.

[pmuellr]

We can actually add /^:/ as an additional element of UNSAFE_HEADERS_PATTERNS and not make the changes here. At least for the new test case, it passes.

[doakalexi]

Updated in this commit, da037c3

[pmuellr]

This approach removes the pseudo-headers from the headers list, so that we don't violate the constraint that the pseudo-headers have to be IN the headers, and FIRST.

From the spec:

All pseudo-header fields MUST appear in a field block before all regular field lines. Any request or response that contains a pseudo-header field that appears in a field block after a regular field line MUST be treated as malformed (Section 8.1.1).

I'm a little worried that the pseudo-headers may end up being required, so an alternative approach would be to leave them, but make sure they're at the beginning of the list of headers. Build a sort function and use a .sort(sortFn) at end of the header array generation.

@JoshMock as our http expert, any thoughts?

[JoshMock]

What are the actual headers that this strips out? If they're needed headers, is there a way to transform them into HTTP/1.1 headers before sending them along?

[pmuellr]

The headers are actually "pseudo-headers". I'm very familiar with the HTTP 1.x specs, but had never heard of these "pseudo-headers" before. See in the HTTP/2 spec 8.3.1. Request Pseudo-Header Fields. When we debugged this, we saw all four listed there, :method, :scheme, :authority, :path.

My guess is that there are used for pipelining, so the data can be "all headers" and no "first line" like HTTP/1, or something.

We are intercepting these from the browser via puppeteer. We have puppeteer open the page that will be "printed", and it gives us a callback for each request sent to the server (main page, css, images, etc). The callback sent us those pseudo-headers, so presumably Chromium headless shell is adding them. This PR removes them in the callback, and things still work, so maybe that's ok? Maybe puppeteer is fixing these up again when we return from the callback? Not sure.

[JoshMock]

Yeah, :method would map to the HTTP method, :path to the HTTP path, etc. If these are passthrough requests that are intended to run as-is on Elasticsearch, you would transform this:

:method GET
:path /foo/bar
:scheme https
:authority my.es.io

to:

GET /foo/bar HTTP/1.1
Host: my.es.io
X-Forwarded-Proto: https

I was suggesting doing that translation rather than dropping the headers entirely, as they may be necessary to properly forward those requests to ES.

[pmuellr]

If these are passthrough requests that are intended to run as-is on Elasticsearch, you would transform this

This is a bit confusing, but my understanding of what's happening is that:

• we make a puppeteer request to load a page
• puppeteer has Chrome makes an http request for each resource
• on those requests, we get some kind of callback that allows us to modify headers (not exactly sure what we are changing in the headers - we just picked up reporting as a new area this year :-) )
• we return the headers we REALLY want Chrome to use when it actually makes the request

So, we're not actually invoking an http client, just intercepting a live request. This happens for all sub-requests as well (images, etc).

I'm not sure we can even do these kind of transforms (of the URL itself, for instance). But I don't think we'd want to.

The question was more of - is it super-important to be sending these to the server anyway? Not sending them, as this PR does, allows reporting to work over http2, which it won't WITHOUT this PR. But I'm thinking long-term we probably DO want to send them. We tried following the rules, putting those "headers" first, and that didn't seem to work. So maybe there's something different we need to do with puppeteer or something.

Current plan is to strip them (merge this PR), since this seems to work for the cases we've tried, for 8.19 and 9.1 (still a few build candidates left before those releases at the end of this month). Better to have something work in at least some cases than fail 100% of the time (for on-prem).

We'll open a separate issue to investigate more to see if we need to do something else for future releases.

[pmuellr]

LGTM

[elasticmachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: dc3447f

Failed CI Steps

• FTR Configs #87

Test Failures

• [job] [logs] FTR Configs #87 / ObservabilityApp Observability alerts > Alerts table Filtering Autocompletion works

Metrics [docs]

✅ unchanged

History

• 💛 Build #315044 was flaky 57d5b0a
• 💚 Build #314274 succeeded 17dba68
• 💔 Build #314248 failed 23296ae

[tsullivan]

LGTM, code review only

[kibanamachine]

Starting backport for target branches: 8.17, 8.18, 8.19, 9.0, 9.1

https://github.com/elastic/kibana/actions/runs/16060406484

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.17
✅	8.18
✅	8.19
✅	9.0
✅	9.1

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

[kibanamachine]

Starting backport for target branches: 8.17, 8.18, 8.19, 9.0, 9.1

https://github.com/elastic/kibana/actions/runs/16061880159

[kibanamachine]

💔 Some backports could not be created

Status	Branch	Result
✅	8.17
✅	8.18
✅	8.19
❌	9.0	Cherrypick failed because the selected commit (3dec3e2) is empty. It looks like the commit was already backported in #226499
✅	9.1

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 225919

Questions ?

Please refer to the Backport tool documentation