[Console] Fix overriding of host header name
#59143
Conversation
Copy the behaviour or Wreck.request from 7.4. This is a regression for setting the Host header value.
jloleysens
added
Feature:Console
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
cjcenizal
left a comment
cjcenizal
left a comment
There was a problem hiding this comment.
Didn't test locally but the code looks good to me overall. Had one suggestion for a way to possibly simplify things. Thanks for fixing this!
| ...finalUserHeaders, | ||
| 'content-type': 'application/json', | ||
| 'transfer-encoding': 'chunked', | ||
| host: hostname, |
There was a problem hiding this comment.
In the past, I've seen the spread object applied after defining default values, as a way to override the defaults with anything defined in the spread object. Would this work here? If so then we won't need finalUserHeaders or hasHostHeader.
headers: {
'content-type': 'application/json',
'transfer-encoding': 'chunked',
host: hostname,
...headers,
},There was a problem hiding this comment.
That solution would work for some cases but unfortunately the override will only take affect if the header object entry name is the same - it could be Host or HoSt or etc.
In the latter case (as in the original issue) both Host and host would be in the headers passed to the endpoint.
|
@jloleysens I removed the 7.6.0 label and added 7.6.2 since that's the next patch release we can ship this with (after 7.6.1 is released today). |
* fix: only override hostname if none was provided Copy the behaviour or Wreck.request from 7.4. This is a regression for setting the Host header value. * Refactor variable name * [skip ci] Fix comment
* fix: only override hostname if none was provided Copy the behaviour or Wreck.request from 7.4. This is a regression for setting the Host header value. * Refactor variable name * [skip ci] Fix comment # Conflicts: # src/plugins/console/server/lib/proxy_request.test.ts # src/plugins/console/server/lib/proxy_request.ts
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* fix: only override hostname if none was provided Copy the behaviour or Wreck.request from 7.4. This is a regression for setting the Host header value. * Refactor variable name * [skip ci] Fix comment # Conflicts: # src/plugins/console/server/lib/proxy_request.test.ts # src/plugins/console/server/lib/proxy_request.ts
* fix: only override hostname if none was provided Copy the behaviour or Wreck.request from 7.4. This is a regression for setting the Host header value. * Refactor variable name * [skip ci] Fix comment
4 participants
Fix #59045
Summary
The
hostheader value is currently being overridden (always in console). This should not be the case and is a regression from 7.4 -> 7.5.The regression was introduced here: #46200
Release Note
We fixed a bug in Console's proxy that would always override the "host" header.
Checklist
For maintainers