Ports for the RESTful API can be specified in two ways; confusion reigns

[tjmcs]

In PR #418 (merged a few weeks ago), a second method was added for specifying the port that the Snap RESTful API listens in on. With the merge of that PR, users can now specify the port using either of the following methods:

• an integer value passed in via the --api-port command-line option or
• a string value of the form ip_address:port passed in via --api-addr command line option (eg. 127.0.0.1:8181)

Both of these methods are also supported in the global configuration file (using the port and addrparameters under the restapi section of that configuration file, which we will refer to as the restapi:port and restapi:addr values, respectively, in the comments to this issue).

While there was code added to try to ensure that users didn't use both of these forms to specify different ports from the command-line, there is no corresponding code in the parsing of the global configuration file to check that this is not the case. As such, users can specify the port as 8080 using the restapi:port value in that global configuration file and specify the port as 8282 by using a value of 127.0.0.1:8282 for the restapi:addr value in that same file.

To make things worse, the code that we've added to check the value passed into Snap for the port the RESTful API will listen on (that the port be greater than zero) won't catch the case where users set the port to zero using the second form (i.e. by setting the port using a restapi:addr or --api-addr value that looks like 127.0.0.1:0). Unfortunately, if both forms are used to specify the port the RESTful API should listen on, the port specified using the restapi:addr or --api-addr value overrides the port specified using the restapi:port or --api-port value, so it's entirely possible for a user to specify an out-of-bounds value for the RESTful API port (one that doesn't match the constraints we've defined for this parameter) by using the restapi:addr or --api-addr form.

We should either disable specifying the port using the second form, i.e. via the ip_address:port form for the restapi:addr or --api-addr value (my personal preference since the port can be specified separately) or we should add code to:

• parse the value passed in via the restapi:addr or --api-addr value to determine if it includes a port
• if it does include a port, use the separate pieces (the IP address and port parsed from that value) to set the values for the IP address and port that the RESTful API will listen on separately, not leave them both embedded in the IP address parameter
• if a port was detected as part of the restapi:addr or --api-addr value, check to make sure that a port was not also specified using a restapi:port or --api-port value; if the port was specified using both of these forms, then the user should be warned that two ports were detected and told which is being used or an error should be thrown indicating that they can't specify the port using both forms.

As I said, my preference would be to remove the ability to specify the port as part of the restapi:addr or --api-addr value (since it's confusing to me to be able to specify it both ways), but other's opinions may vary.

[jcooklin]

@obourdon: would you like to weigh in here?

[obourdon]

I will try to get a deeper look into this issue soon and will hopefully come back with more infos. Thanks for the notice

[obourdon]

OK, having looked at this more deeply I have seen some areas of improvement going your way.

However I also have a tendency to also make sure the user understands that he has to be knowledgeable and accountable for what he is doing. Having both a configuration file and command line parameters is always dangerous and the usual way to treat this is by having command line values to take precedence over config file values which in turn supersede code internal defaults.

This is furthermore emphasized by the comments in the code existing prior to my PR:
see these lines

As far as making sure that the supplied values comply with constraints lines I think I can forge out something sometimes tomorrow.

Any other advices ???

[tjmcs]

We already have code in place to ensure the command-line parameters override the same parameters if they are specified in the global configuration file, @obourdon, the issue here is if the port is specified as part of the --api-add and using the --api-port (or as part of the restapi:addr and using the restapi:port, or any combination of those 4 parameters). If the port is included as part of the RESTful API address, then that overrides any port specified as using either the restapi:port or --api-port values, which is bad enough, but even worse a port specified using the RESTful API address is not checked for consistency with the constraints that are applied to the same value of it's specified using a restapi:port or --api-port value. That's the issue here, not an issue of whether the values are specified via the CLI or the configuration file.