Elastica icon indicating copy to clipboard operation
Elastica copied to clipboard

Allow base url without ending slash

Open jaroslavtyc opened this issue 2 years ago • 3 comments

Accept both with and without trailing slash URL as Elastic source.

I have spent few time to find out what is wrong with Elastic URL https://foo:[email protected], getting error Couldn't resolve host

./bin/console fos:elastica:create

Creating bitbag_shop_product

In Http.php line 186:
                         
  Couldn't resolve host  
                         

fos:elastica:create [--index [INDEX]] [--no-alias]

Reason was missing trailing slash in URL, leading into invalid URL https://foo:[email protected]_shop_product instead of correct https://foo:[email protected]/bitbag_shop_product

Suggested change ensures slash between base URL and request path (Elastic index name in this case).

jaroslavtyc avatar May 16 '23 11:05 jaroslavtyc

This reminds me of https://github.com/ruflin/Elastica/issues/1777 Can you check where we landed there? Need to read up on it again.

ruflin avatar May 17 '23 09:05 ruflin

@ruflin In that issue I have found opposite problems

  1. it is broken without trailing slash https://github.com/ruflin/Elastica/issues/1777#issuecomment-735692398
  2. it is broken with trailing slash https://github.com/ruflin/Elastica/issues/1777#issue-617992054

My proposal is to make sure there is slash if and only if there will be path appended to host. It can be redundant if you solve somehow the https://github.com/ruflin/Elastica/issues/1777, but it will solve without any detect-server-and-client-expectations magic single problem now and will be harmless later.

jaroslavtyc avatar May 17 '23 11:05 jaroslavtyc

Thanks for the clarification. I couldn't really think of a reason why there would ever be no / be needed in this situation except the path put in would start with a /. But then again, users don't put paths in but indexNames for example where / is not an allowed character. But now things become interesting: All tests fail so there seems to be a pretty strong side effect of this. Can you take a closer look on what happens?

Please also add a changelog entry.

ruflin avatar May 26 '23 06:05 ruflin