Make port coercion slightly more compatible with urls ending in /api/

[erpel]

This pull request tries to adress some problematic server-url arguments.

Before, URLs for servers, that do not use a separate port for the REST API did not work at all, because the coerceOptions function would append :12900 after the /api/ part in a url like http://graylog-server.internal:9000/api.

With this pull request, such URLs would be left unchanged by the port coercion mechanism. It unfortunately does not address URLs which omit the port but end in / for example, but this at least makes it possible to use the dashboard with servers that do not use separate ports for the REST API at all.

[CLAassistant]

All committers have signed the CLA.

[STRML]

Yeah, I think the far better solution is for us to separate this into host, port, and path.

[tholu]

@STRML Are you planning on fixing this soon by separating this into host, port, and path? I wanted to test it today and ran directly into this issue.

[STRML]

Haven't gotten to it yet. If you have a chance please submit a PR.

On Aug 8, 2017 12:45 PM, "Thomas Lutz" notifications@github.com wrote:

@STRML https://github.com/strml Are you planning on fixing this soon by separating this into host, port, and path? I wanted to test it today and ran directly into this issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/graylog-labs/cli-dashboard/pull/30#issuecomment-321030356, or mute the thread https://github.com/notifications/unsubscribe-auth/ABJFP5a7u6H3YNJEtjx8e0gcxCwpEFHEks5sWJ6_gaJpZM4Op7-F .

[erpel]

Node also has url parsing functionality apparently: https://nodejs.org/docs/latest/api/url.html. Maybe using something like that might make for a cleaner interface compared with 3 awkward separate parameters.

I'm not sure what the most commonly used setups are, but just using defaults in line with what most other programs do when confronted with http urls might be the path of least astonishment. Chances are you know what the url needs to be. I feel there is something to be said for using the reserved port for the given protocol. Defaulting to https/443 when no schema is given might be acceptable today, encouraging a secure default.

[STRML]

I'm reworking this now, I think it makes a cleaner interface overall to just split it up; by setting sensible port, path, protocol defaults it's generally going to be as simple as --api-host=graylog.foo.com for configuration.

[STRML]

Fixed in 435e664 - thanks for the contribution.

[tholu]

@STRML Thanks for this fast fix! I actually fixed it locally with the URL parsing module similar to what @erpel suggested and wanted to open a PR, but you beat me and I like your solution as well.