Merge lp:~er-abhinav-upadhyay/ubuntu/natty/tomcat6/bug707405 into lp:ubuntu/natty/tomcat6

[Abhinav Upadhyay]

tomcat6-instance-create should accept -1 as the value of -c option
  as per http://tomcat.apache.org/tomcat-6.0-doc/config/server.html (LP: #707405)

As per the bug #707405, tomcat6-instance-create should accept -1 as the value of the -c option.

I have modified the tomcat6-instance-create script so that it will now accept -1 as a value for -c option. It will configure the server alright, but it will give the user a warning message, that he will have to kill the server manually.

[Clint Byrum (clint-fewbar)]

As an alternative one could use this for number detection:

if ! echo $port | grep -Eq '^\-?[0-9]*$' ; then

Advantages:
 * more obvious match of exact *type* of number expected (integers only with optional leading minus)
 * no redirect of errors to /dev/null (in case something else goes wrong not involving the failure you're trying to trigger)

[Barry Warsaw (barry)]

Hi Abhinav,

Thanks for taking the time to improve Ubuntu!

Shouldn't that regexp be:

'^\-?[0-9]+$'

IOW, there must be at least one digit? Otherwise '-' alone will match, but it's not a number so the -eq will fail later on.

Other than that, I think Clint's approach makes sense. Would you consider making this change?

review: Needs Fixing

[Abhinav Upadhyay (er-abhinav-upadhyay)]

> Hi Abhinav,
>
> Thanks for taking the time to improve Ubuntu!
>
> Shouldn't that regexp be:
>
> '^\-?[0-9]+$'
>
> IOW, there must be at least one digit? Otherwise '-' alone will match, but
> it's not a number so the -eq will fail later on.
>
> Other than that, I think Clint's approach makes sense. Would you consider
> making this change?

Yes, I am making the changes and will be pushing the changes.
Thanks for review it :-)

[lp:~er-abhinav-upadhyay/ubuntu/natty/tomcat6/bug707405]

34. By Abhinav Upadhyay on 2011-03-08

tomcat6-instance-create: Now, using a regex test to check if port numbers are valid numbers or not.

[Abhinav Upadhyay (er-abhinav-upadhyay)]

Hi,
Thanks to both of you for review my merge proposal :-)

I have replaced the previous port number test with the regex test as both of you had suggested. I also tested it with all kinds of inputs, and it worked ok.
I request for another review of the patch.

Thanks
Abhinav

[Dave Walker (davewalker)]

Thanks for fixing this and improving the change based on comments. I have now uploaded it, so thank you for your contribution.

I spoke with one of the Debian maintainers of this package the other day, and he agreed with fixing it. I don't believe you've submitted it there, so doing that now.

Thanks.

review: Approve

[Abhinav Upadhyay (er-abhinav-upadhyay)]

Dave: Thanks a lot for reviewing and approving it. Yes, I did not submit it upstream, thank you for doing that as well. :-)

=== modified file 'debian/changelog'
--- debian/changelog	2011-02-09 21:49:33 +0000
+++ debian/changelog	2011-03-08 04:17:10 +0000
@@ -1,3 +1,10 @@
+tomcat6 (6.0.28-10ubuntu1) natty; urgency=low
+
+  * tomcat6-instance-create should accept -1 as the value of -c option
+    as per http://tomcat.apache.org/tomcat-6.0-doc/config/server.html (LP: #707405)
+  
+ -- Abhinav Upadhyay <[e-mail]>  Mon, 07 Mar 2011 13:38:05 +0530
+
 tomcat6 (6.0.28-10) unstable; urgency=medium
 
   * Team upload.
 
=== modified file 'debian/tomcat6-instance-create'
--- debian/tomcat6-instance-create	2010-05-21 13:51:15 +0000
+++ debian/tomcat6-instance-create	2011-03-08 04:17:10 +0000
@@ -23,11 +23,22 @@
   type=$1
   port=$2
   # Fail if port is non-numeric
-  num=`expr ${port} + 1 2> /dev/null`
-  if [ $? != 0 ] || [ $num -lt 2 ]; then
+  if ! echo $port | grep -Eq '^\-?[0-9]+$' ; then
     echo "Error: ${type} port '${port}' is not a valid TCP port number."
     exit 1
   fi
+  
+  # If Control port is -1 , no need to check any further. 
+  if [ "$type" = "Control" ] && [ $port -eq -1 ]; then
+  	echo "Warning: Control port disabled. You will have to shutdown the server manually, by using OS signals."
+  	return
+  fi
+  
+  # Fail if port is 0 or negative 
+  if [ $port -le 0 ]; then
+  	echo "Error: ${type} port '${port}' is not a valid TCP port number."
+  	exit 1
+  fi
 
   # Fail if port is above 65535
   if [ ${port} -gt 65535 ]; then