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

Specifying ssl => true leads to exception "URI::InvalidURIError: bad URI(is not URI?)" #102

Closed
webmat opened this issue Jun 20, 2018 · 6 comments
Assignees

Comments

@webmat
Copy link
Contributor

webmat commented Jun 20, 2018

When specifying ssl => true (with or without ca_cert param), the URL string is not interpolated correctly. Here's the error raised:

[2018-06-20T14:18:43,541][WARN ][logstash.filters.elasticsearch] Failed to query elasticsearch for previous event {:index=>"cert", :query=>"@timestamp:*", :event=>#<LogStash::Event:0x7ad689eb>, :error=>#<URI::InvalidURIError: bad URI(is not URI?): https://{:host=>"https://example.us-central1.gcp.cloud.es.io:9243/", :scheme=>"https"}:https>}

Notice the URL string appears to contain the full params hash (hash.to_s): https://{:host=>"https://example.us-central1.gcp.cloud.es.io:9243/", :scheme=>"https"} instead of templating the fields in a string template (e.g. "%{scheme}://%{host}" % { scheme: 'https', host: '...' })

This particular trace was generated by the following configuration (subdomain of the cloud instance changed):

bin/logstash --log.level debug -e "input { generator { count =>  3 } }
filter { elasticsearch {
  user => elastic password => '$ES_PWD' hosts => ['https://example.us-central1.gcp.cloud.es.io:9243/']
  index => 'cert' query => '@timestamp:*' sort => '_id' fields => { 'sequence' => 'last_sequence' }
  ssl => true
} }
output { stdout {} elasticsearch {
  user => elastic password => '$ES_PWD' hosts => ['https://example.us-central1.gcp.cloud.es.io:9243/']
  index => 'cert'
} }"

Known workaround

Specify all hosts with their https:// prefix and do not specify the ssl attribute.

Full stack trace

#<URI::InvalidURIError: bad URI(is not URI?): https://{:host=>"https://example.us-central1.gcp.cloud.es.io:9243/", :scheme=>"https"}:https>

 "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/uri/rfc3986_parser.rb:67:in `split'",
 "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/uri/rfc3986_parser.rb:73:in `parse'",
 "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/uri/common.rb:227:in `parse'",
 "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/uri/common.rb:714:in `URI'",
 "org/jruby/RubyMethod.java:127:in `call'",
 "/Users/mat/work/elastic/logstash/vendor/bundle/jruby/2.3.0/gems/faraday-0.9.2/lib/faraday/utils.rb:258:in `URI'",
 "/Users/mat/work/elastic/logstash/vendor/bundle/jruby/2.3.0/gems/faraday-0.9.2/lib/faraday/connection.rb:309:in `url_prefix='",
 "/Users/mat/work/elastic/logstash/vendor/bundle/jruby/2.3.0/gems/faraday-0.9.2/lib/faraday/connection.rb:77:in `initialize'",
 "/Users/mat/work/elastic/logstash/vendor/bundle/jruby/2.3.0/gems/elasticsearch-transport-5.0.4/lib/elasticsearch/transport/transport/http/faraday.rb:38:in `__build_connection'",
 "/Users/mat/work/elastic/logstash/vendor/bundle/jruby/2.3.0/gems/elasticsearch-transport-5.0.4/lib/elasticsearch/transport/transport/base.rb:138:in `block in __build_connections'",
 "org/jruby/RubyArray.java:2486:in `map'",
 "/Users/mat/work/elastic/logstash/vendor/bundle/jruby/2.3.0/gems/elasticsearch-transport-5.0.4/lib/elasticsearch/transport/transport/base.rb:130:in `__build_connections'",
 "/Users/mat/work/elastic/logstash/vendor/bundle/jruby/2.3.0/gems/elasticsearch-transport-5.0.4/lib/elasticsearch/transport/transport/base.rb:40:in `initialize'",
 "/Users/mat/work/elastic/logstash/vendor/bundle/jruby/2.3.0/gems/elasticsearch-transport-5.0.4/lib/elasticsearch/transport/client.rb:114:in `initialize'",
 "/Users/mat/work/elastic/logstash/vendor/bundle/jruby/2.3.0/gems/elasticsearch-transport-5.0.4/lib/elasticsearch/transport.rb:26:in `new'",
 "/Users/mat/work/elastic/plugins/logstash-filter-elasticsearch/lib/logstash/filters/elasticsearch/client.rb:28:in `initialize'",
 "/Users/mat/work/elastic/plugins/logstash-filter-elasticsearch/lib/logstash/filters/elasticsearch.rb:145:in `new_client'",
 "/Users/mat/work/elastic/plugins/logstash-filter-elasticsearch/lib/logstash/filters/elasticsearch.rb:149:in `block in get_client'",
 "/Users/mat/work/elastic/plugins/logstash-filter-elasticsearch/lib/logstash/filters/elasticsearch.rb:149:in `get_client'",
 "/Users/mat/work/elastic/plugins/logstash-filter-elasticsearch/lib/logstash/filters/elasticsearch.rb:93:in `filter'",
 "/Users/mat/work/elastic/logstash/logstash-core/lib/logstash/filters/base.rb:145:in `do_filter'",
 "/Users/mat/work/elastic/logstash/logstash-core/lib/logstash/filters/base.rb:164:in `block in multi_filter'",
 "org/jruby/RubyArray.java:1734:in `each'",
 "/Users/mat/work/elastic/logstash/logstash-core/lib/logstash/filters/base.rb:161:in `multi_filter'",
 "/Users/mat/work/elastic/logstash/logstash-core/lib/logstash/filter_delegator.rb:44:in `multi_filter'",
 "(eval):46:in `block in filter_func'",
 "/Users/mat/work/elastic/logstash/logstash-core/lib/logstash/pipeline.rb:443:in `filter_batch'",
 "/Users/mat/work/elastic/logstash/logstash-core/lib/logstash/pipeline.rb:422:in `worker_loop'",
 "/Users/mat/work/elastic/logstash/logstash-core/lib/logstash/pipeline.rb:384:in `block in start_workers'"
@webmat
Copy link
Contributor Author

webmat commented Jun 20, 2018

The problem appears to be with this line. It replaces URL strings with ruby hashes.

In my example, before:

{:hosts=>["https://example.us-central1.gcp.cloud.es.io:9243/"]}

and after this line

{:hosts=>[{:host=>"https://example.us-central1.gcp.cloud.es.io:9243/", :scheme=>"https"}]}

@webmat webmat self-assigned this Jun 20, 2018
@webmat
Copy link
Contributor Author

webmat commented Jun 28, 2018

Currently setting ssl => true is completely broken in this plugin. I'm adding tests to describe the desired behaviour. However the full rewrite of the HTTP subsystem we want to do, to bring all 3 ES plugins in line is out of scope for this bug fix.

Here's how I intend to make it behave, after this bug fix.

    it "should accept schema-less hosts"
    it "should accept hosts with a schema"
    it "should add https to schema-less hosts when specifying ssl true"
    it "should log an error and raise, when specifying a schema in at least one host, and setting ssl true"
    it "should log an error and raise, when specifying a schema in at least one host, and setting ssl false"

Note that the last two are a bit more strict than the behaviour of output-elasticsearch. Specifying any schema in the host list, then setting ssl (which is meant to add a schema everywhere for them) is simply a user passing conflicting args.

I don't think we should jump through hoops to try to accomodate this behaviour. The user should either specify schema for all hosts, or not specify them for any. Then if you set ssl => true we add https:// for you (adding ssl => false doesn't need to do anything, ES::Client will default to http).

If someone needs to specify a mixed set of SSL and not SSL, it's just as simple: specify your http and https schemas in the host list and don't touch the ssl attribute.

Of course the error will make it clear that they should use one or the other, not both. Same for the documentation for ssl.

WDYT, @jsvd @robbavey?

@andrewbanchich
Copy link

I can confirm I've been getting this as well.

@nilskuhn
Copy link

nilskuhn commented Dec 3, 2019

This bug cost me some hours :(

One suggestion and one please I have:

Suggestion
Could you please remove ssl option from official documentation or better document the problem with it there.

Question
However the full rewrite of the HTTP subsystem we want to do, to bring all 3 ES plugins in line is out of scope for this bug fix.
Can I see new plugin versions already somewhere as an alpha or beta?

Best regards, Nils

@breml
Copy link

breml commented Jan 11, 2021

Issue #129 is related to this as well.

@edmocosta
Copy link
Contributor

Thank you very much for taking the time to open this issue.

I'm closing it as It seems to be already fixed by this PR #156. Please feel free to reopen if the problem still exists. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants