Fix listen address processing

[ckuethe]

Whatever skin it's wearing InterSpec is a web app. It smells wrong to me to change the function signature of start_server and friends depending on whether BUILD_FOR_WEB_DEPLOYMENT or BUILD_AS_LOCAL_SERVER are defined, when all the same data files are being used and all javascript and HTML is being served no matter what display engine is being used.

This diff does a few things:

• makes the function arguments and CLI parameters consistent whether or not BUILD_FOR_WEB_DEPLOYMENT is defined. The http-address parameter always exists now.
• sets a sensible default value of "127.0.0.1". Unless you know what you're doing and why, you probably intend to access InterSpec only from the local machine. And unless you specifically give a non-local bind address, InterSpec will only listen on localhost.
• Makes --http-address optional, and uses the localhost default if not otherwise specified
• updates help text to reflect these changes

Tested with

• B_F_W_D/B_A_L_S
• localhost/remote client
• 127.0.0.1, 0.0.0.0, my real IP, a second IP on this machine, the hostname of this machine, a hostname that doesn't exist, an IP that doesn't exist on this machine, an invalid IP

[wcjohns]

Thanks for this Chris!

I'm a bit torn on one part though: I agree it seems a little wrong to have a different function signature of startWebServer, depending if its meant for localhost vs web-serving; however, it also seems about as wrong to instead ignore a value of the http_address argument when BUILD_FOR_WEB_DEPLOYMENT is not defined. The reason I originally went for the different signatures, was to extra make sure that for any build that might be made external to your computer (there are a few different use cases for this), you had to be explicit over how things were initialized (since files outside of the InterSpec deployment could potentially be accessed), and this was really what you wanted the code to do, relative to localhost builds.

Also, the BUILD_FOR_WEB_DEPLOYMENT=ON option is not something that I have used or tested for a couple years now, so please use caution.

I have a busy next few days, so I'll think some more on this, and come back to this.

again, thank you!

[ckuethe]

No worries, I didn't think this diff would land right away. It just seems easier to reason about the purpose and future of BUILD_FOR_WEB_DEPLOYMENT if there is a diff to discuss.

For the local deployment case, I'll rework the logic to not ignore the http-address parameter and instead loudly enforce a requirement that it somehow resolve to localhost (ie. leave it unspecified to use the localhost default, or explicitly say "localhost" or "127.0.0.1"). In other words, if you provide a not-localhost address to a localhost build, it will fail to start and tell you why. I saw the docker target which is what got me started down this rabbit hole in the first place.

Data sharing/isolation/security is on my mind. Single-user-multi-display is my primary use case for web deployment, but I know some other people who also think gamma ray spectroscopy is fun. For now, they can install interspec on their devices and can all work out of a shared drive. It might be useful if we could have a central interspec instance backed by some shared storage. I assume there are still other interspec users who turned pale and shouted "oh heck no!" at the thought of exposing their spectra and analysis on the internet. In a way this isn't too different from Jupyter. I run jupyter on my laptop for quick prototypes, and on my workstation where I have more resources. I could put a reverse proxy in front of that so I could hit it remotely. And I've seen jupyter deployed in business setting using various auth and isolation models from a single host with a single shared password, all the way up to dockerized with a container per user and auth via SSO.

Again, this is for discussion and there's no need to merge this urgently. I'd rather take the time to figure out the right way than to quickly ship bugs.

[wcjohns]

Sorry its taken so long to get to this request.

I wasn't ignoring it - I was thinking about the how the network adapter address to bind to is specified. Making the call to InterSpecServer::start_server(...) consistent wether it is being deployed as a web-app or not does clean stuff up in some ways. However, you are then left with a parameter that gets a runtime check that its value is exactly the one value that is allowed. I am always afraid of critical runtime checks getting broken in the future, on accident. And it moves catching a mistake (invalid bind address used) from compile-time, to runtime, which I'm slightly not in favor of.

So I think I would prefer to leave the function signature as changing, depending on BUILD_FOR_WEB_DEPLOYMENT.

However, this pull request does fix some definite bugs in checking for configuration parameters being specified when built for web deployment. E.x., if( cl_vm.count("config") ) should be if( 0 == cl_vm.count("config") ), as you have fixed. And I like the information message you added to stdout.

Would you be willing to submit a pull request that fixes these bugs, but doesn't change the start_server(...) function signature?
If not, or you would prefer, I will be happy to manually apply your bug fixes in this request.

Thank you for these fixes, and this discussions,
-will

[ckuethe]

Thanks for the feedback, I'll have an updated PR for you shortly.