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

Internal server error when saving a widget with German characters in name #8705

Closed

Conversation

weltenwort
Copy link
Member

@weltenwort weltenwort commented Oct 19, 2016

Kibana version:
5.0-rc2 snapshot, build 14376

Elasticsearch version:
5.0-rc1

Server OS version:
MacOS, El Capitan 10.11.6 (15G31)

Browser version:
Version 53.0.2785.143 (64-bit)

Browser OS version:
MacOS, El Capitan 10.11.6 (15G31)

Original install method (e.g. download page, yum, from source, etc.):
Manual tar

Description of the problem including expected versus actual behavior:
When creating widgets with special German characters (and m

Steps to reproduce:

  1. Create any widget
  2. Set name to something that has an umlaut (üäö) or German ß
  3. Get internal server error

Errors in browser console (if relevant):

Error: Request to Elasticsearch failed: "Internal Server Error"
    at http://localhost:5601/bundles/kibana.bundle.js?v=14376:26:14736
    at processQueue (http://localhost:5601/bundles/commons.bundle.js?v=14376:38:23621)
    at http://localhost:5601/bundles/commons.bundle.js?v=14376:38:23888
    at Scope.$eval (http://localhost:5601/bundles/commons.bundle.js?v=14376:39:4619)
    at Scope.$digest (http://localhost:5601/bundles/commons.bundle.js?v=14376:39:2359)
    at Scope.$apply (http://localhost:5601/bundles/commons.bundle.js?v=14376:39:5037)
    at done (http://localhost:5601/bundles/commons.bundle.js?v=14376:37:25027)
    at completeRequest (http://localhost:5601/bundles/commons.bundle.js?v=14376:37:28702)
    at XMLHttpRequest.xhr.onload (http://localhost:5601/bundles/commons.bundle.js?v=14376:37:29634)

Error from Kibana log:

 error  [15:08:01.633]  Error: Uncaught error: Header value cannot contain or convert into non-ascii characters: location
    at Object.exports.assert (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/node_modules/hoek/lib/index.js:736:11)
    at internals.Response._header (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/response.js:141:22)
    at internals.Response.header (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/response.js:119:17)
    at internals.Response._passThrough (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/response.js:502:26)
    at internals.Response._prepare (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/response.js:457:10)
    at process.nextTick (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/reply.js:152:22)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickDomainCallback (internal/process/next_tick.js:122:9)
Debug: internal, implementation, error 
    Error: Uncaught error: Header value cannot contain or convert into non-ascii characters: location
    at Object.exports.assert (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/node_modules/hoek/lib/index.js:736:11)
    at internals.Response._header (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/response.js:141:22)
    at internals.Response.header (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/response.js:119:17)
    at internals.Response._passThrough (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/response.js:502:26)
    at internals.Response._prepare (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/response.js:457:10)
    at process.nextTick (/Users/tanya/kibana-5.0.0-rc2-SNAPSHOT-darwin-x86_64/node_modules/hapi/lib/reply.js:152:22)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickDomainCallback (internal/process/next_tick.js:122:9)

Interestingly, visualization is still saved on refresh.

screen shot 2016-10-16 at 8 07 58 am

screen shot 2016-10-16 at 8 07 35 am

screen shot 2016-10-16 at 8 00 57 am

screen shot 2016-10-16 at 8 01 02 am

Note: It's possible other languages and special characters are affected... I tested Russian, Japanese, and Bengali and didn't run into the same problem, but it was not thorough testing.

@tbragin tbragin added bug Fixes for quality problems that affect the customer experience v5.0.0 labels Oct 16, 2016
@weltenwort weltenwort self-assigned this Oct 17, 2016
@weltenwort
Copy link
Member

According to my analysis this error is caused by missing url encoding of the Location header in the response by ElasticSearch, which Kibana tries to forward to the browser. This fails because hapi < 15.0.0 applies strict header validation that only allows 7-bit encoded strings. From version 15.0.0 on hapi has removed that validation in favor of Node.js's less restrictive built-in validation (see hapijs/hapi#3065).

Possible workarounds and solutions would be to...

  1. update to hapi >= 15.0.0 (probably feasable mid-term)
  2. uri-encode the Location header manually before sending it on (quick workaround)
  3. uri-encode the Location header in ElasticSearch before sending it

@weltenwort
Copy link
Member

weltenwort commented Oct 18, 2016

To add a few more details about the Location header as returned by ElasticSearch: Characters with code <= 255 seem to be returned as-is without encoding and everything else seems to be replaced with a ?.

@spalger
Copy link
Contributor

spalger commented Oct 18, 2016

@weltenwort I would rather not transform the headers coming back from Elasticsearch, but if @tylersmalley had a good reason for not going to 15+ it might be necessary. Either way it would be great if you would file an issue at elastic/elasticsearch.

@weltenwort
Copy link
Member

@spalger I totally agree with header rewriting not being a great solution. But if we want this to be fixed before 5.0.0 GA I would feel apprehensive of making as big a change as upgrading to hapi.

@tylersmalley
Copy link
Contributor

I agree with @weltenwort that upgrading to Hapi 15 would be a large change to get in at the last minute before GA. I will work on the upgrade path and see what is necessary. @weltenwort, do you want to go down the rewrite path?

@kobelb
Copy link
Contributor

kobelb commented Oct 18, 2016

It would also theoretically be possible for us to fork hapijs@14 and cherry-pick this commit onto it, if the rewriting path doesn't work out hapijs/hapi@9f2bd3d

I tried this out locally, and it seemed to resolve the issue.

@weltenwort
Copy link
Member

Yes, that's another possibility. I have a re-encode PR almost done though - just want to add some tests. Thanks for all your input! 🙇‍♂️

@tylersmalley
Copy link
Contributor

I have a PR up, but still have quite a bit of testing to do: #8737

Right now there are situations in which ElasticSearch puts characters in
the character code range between 128 and 255 into the `Location` header.
This leads to an exception when trying to pass on that header through
the hapi proxy in versions before 15.0.0, because it validates that only
US-ASCII characters are used in headers.

To work around that situation, the `Location` header is encoded using
`encodeURI()` for now.

Closes elastic#8705
Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jbudz jbudz self-assigned this Oct 19, 2016
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@@ -20,7 +20,14 @@ function createProxy(server, method, route, config) {
xforward: true,
timeout: server.config().get('elasticsearch.requestTimeout'),
onResponse: function (err, responseFromUpstream, request, reply) {
reply(err, responseFromUpstream);
const upstreamLocation = responseFromUpstream.headers.location;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can depend on responseFromUpstream being defined if there was an error. We should probably early return if there was an error and then read the location header from the responseFromUpstream

elastic-jasper added a commit that referenced this pull request Oct 19, 2016
---------

**Commit 1:**
URI-encode forwarded location header in proxy

Right now there are situations in which ElasticSearch puts characters in
the character code range between 128 and 255 into the `Location` header.
This leads to an exception when trying to pass on that header through
the hapi proxy in versions before 15.0.0, because it validates that only
US-ASCII characters are used in headers.

To work around that situation, the `Location` header is encoded using
`encodeURI()` for now.

Closes #8705

* Original sha: 18c23c1
* Authored by Felix Stürmer <[email protected]> on 2016-10-18T17:55:31Z

**Commit 2:**
Add test to verify umlaut in vis name

Relates to #8705

* Original sha: e100e1f
* Authored by Felix Stürmer <[email protected]> on 2016-10-19T09:01:46Z

**Commit 3:**
[elasticsearch/proxy] use different code path with erorr

* Original sha: fec5e1a
* Authored by spalger <[email protected]> on 2016-10-19T19:06:39Z
elastic-jasper added a commit that referenced this pull request Oct 19, 2016
---------

**Commit 1:**
URI-encode forwarded location header in proxy

Right now there are situations in which ElasticSearch puts characters in
the character code range between 128 and 255 into the `Location` header.
This leads to an exception when trying to pass on that header through
the hapi proxy in versions before 15.0.0, because it validates that only
US-ASCII characters are used in headers.

To work around that situation, the `Location` header is encoded using
`encodeURI()` for now.

Closes #8705

* Original sha: 18c23c1
* Authored by Felix Stürmer <[email protected]> on 2016-10-18T17:55:31Z

**Commit 2:**
Add test to verify umlaut in vis name

Relates to #8705

* Original sha: e100e1f
* Authored by Felix Stürmer <[email protected]> on 2016-10-19T09:01:46Z

**Commit 3:**
[elasticsearch/proxy] use different code path with erorr

* Original sha: fec5e1a
* Authored by spalger <[email protected]> on 2016-10-19T19:06:39Z
nreese pushed a commit to nreese/kibana that referenced this pull request Nov 10, 2016
Right now there are situations in which ElasticSearch puts characters in
the character code range between 128 and 255 into the `Location` header.
This leads to an exception when trying to pass on that header through
the hapi proxy in versions before 15.0.0, because it validates that only
US-ASCII characters are used in headers.

To work around that situation, the `Location` header is encoded using
`encodeURI()` for now.

Closes elastic#8705
nreese pushed a commit to nreese/kibana that referenced this pull request Nov 10, 2016
@epixa epixa added v5.1.1 and removed v5.1.0 labels Dec 8, 2016
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
---------

**Commit 1:**
URI-encode forwarded location header in proxy

Right now there are situations in which ElasticSearch puts characters in
the character code range between 128 and 255 into the `Location` header.
This leads to an exception when trying to pass on that header through
the hapi proxy in versions before 15.0.0, because it validates that only
US-ASCII characters are used in headers.

To work around that situation, the `Location` header is encoded using
`encodeURI()` for now.

Closes elastic#8705

* Original sha: 03ccb8ec56eb92c86162e307a286305384ba92c3 [formerly 18c23c1]
* Authored by Felix Stürmer <[email protected]> on 2016-10-18T17:55:31Z

**Commit 2:**
Add test to verify umlaut in vis name

Relates to elastic#8705

* Original sha: 238f6f64a55d113f30fd8f61f5330c1c97f23627 [formerly e100e1f]
* Authored by Felix Stürmer <[email protected]> on 2016-10-19T09:01:46Z

**Commit 3:**
[elasticsearch/proxy] use different code path with erorr

* Original sha: 0f7e4548c7808d845bf0df661d9a3742b81f14da [formerly fec5e1a]
* Authored by spalger <[email protected]> on 2016-10-19T19:06:39Z


Former-commit-id: afbc790
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience review v5.0.0 v5.1.1 v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants