[Nominatim] Set optional parameters of http_build_query

[sebalis]

If arg_separator is not set, the default value comes from arg_separator.output which is not always set to the value we need here, which is '&'. On Ubuntu the default is '&'. There is no need to adhere to any system defaults here because we know exactly in which context the URLs we build are used: for a backend request that is executed soon in the same PHP handler. The same parameters are used in the MapQuest provider.

[jbelien]

Hello @sebalis ,

Thank you for your contribution!

I must admit I'm a bit "confused" by this ... I actually don't think it's actually an issue since all our tests are running on Ubuntu and I'm also running on Ubuntu without having any issue. I just checked my PHP configuration (Ubuntu 20.04 with PHP 7.4 & 8.0):

; The separator used in PHP generated URLs to separate arguments. ; PHP's default setting is "&". ; http://php.net/arg-separator.output ; Example: ;arg_separator.output = "\&"

So if in your configuration arg_separator.output is set to &, it's most likely a custom configuration and you're not using PHP's default setting.

PS: Nominatim tests are currently failing, I'll need to check why.

[sebalis]

Hi @jbelien, thanks for responding!

I must admit that I was not sure myself why the constant is set to & on the Ubuntu instance where I encountered the error. I trired to find out which package installs /etc/php/*/*/php.ini and didn’t find one. Then I found that Ububtu ships suggested files for php.ini in the php7.4-common package, which installs them under /usr/lib/php/7.4/ as php.ini-{development,prroduction,production.cli}. In all these files, arg_separator.output is set to "&". There seems to be an upstream source in php7.4: it contains an archive php7.4_7.4.3.orig.tar.xz which has two of these three files, both with the same value.

But my argument goes beyond this. The fact that calling http_build_query without setting these optional arguments introduces a flexibility that we don’t want. We know exactly which syntax we want to use for the URLs constructed in this file as we are going to use them for opening a server-to-server connection to Nominatim in the backend, straight after they are constructed. Getting those ini constants involved clearly serves another purpose – where the string is first written somewhere (say, as part of textual or HTML output) and then only used by some other process that works on this output (such as a browser). What is a welcome flexibility there is nothing but unwelcome uncertainty in this context. Let me again point to the MapQuest provider, which uses these same arguments (that is actually where I took them from).

[sebalis]

I’ve only just realised that you seem to think using & is correct. In my situation it wasn’t – I needed & to make my queries to nominatim.openstreetmap.org succeed. That’s why I used that value in the parameters I am proposing to add.

Edit: No, sorry, you are not thinking that & is correct, you’re saying it’s commented out in a default php.ini. And you’re right – I completely overlooked that. Which means that I still don’t know how & got to be used on my server. But the general argument still applies: No matter which value arg_separator.output may have, we should not use the system’s configuration in this context because we do not want this variability. And I have seen (by adding debug output) that the ini value was set to & on my system and patching my installation to add these arguments solved the problem for me. I didn’t want to change the ini value because I didn’t know if it might be needed elsewhere (it is not really “my” system)

[jbelien]

Thanks for the reminder, it slipped my mind.

[sebalis]

Thanks – I had wondered that maybe I should have created an issue together with the PR straight away :-)

[jbelien]

No, no, it was not necessary. I just forgot about it.

I'll fix the tests and create a new release ASAP.