Bug 177944 - news/sabnzbdplus rc script breaks on non-ip host values
Summary: news/sabnzbdplus rc script breaks on non-ip host values
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Mark Felder
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-18 05:00 UTC by will
Modified: 2013-12-05 19:40 UTC (History)
0 users

See Also:


Attachments
sabnzbd-broken-rc-script.patch (963 bytes, patch)
2013-04-18 05:00 UTC, will
no flags Details | Diff
177944.diffv3 (6.07 KB, application/octet-stream)
2013-04-23 16:44 UTC, Mark Felder
no flags Details
177944.diffv2 (6.09 KB, application/octet-stream)
2013-04-23 16:40 UTC, Mark Felder
no flags Details
file.dat (1 bytes, multipart/related; boundary="------------030800000301080309070409")
2013-04-23 17:09 UTC, will
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description will 2013-04-18 05:00:00 UTC
	The egrep regexp for sabnzbd's init script doesn't allow for non-IPv4 address values. Since hostnames and IPv6 addresses are valid options for the host field in the configuration file, I've submitted a patch that matches any host value, and set ${host} to it appropriately

Fix: attached patch
How-To-Repeat: 	stop sabnzbd with the host field in sabnzbd.ini set to a non-IPv4 address
Comment 1 Edwin Groothuis freebsd_committer freebsd_triage 2013-04-18 05:50:44 UTC
Maintainer of news/sabnzbdplus,

Please note that PR ports/177944 has just been submitted.

If it contains a patch for an upgrade, an enhancement or a bug fix
you agree on, reply to this email stating that you approve the patch
and a committer will take care of it.

The full text of the PR can be found at:
    http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/177944

-- 
Edwin Groothuis via the GNATS Auto Assign Tool
edwin@FreeBSD.org
Comment 2 Edwin Groothuis freebsd_committer freebsd_triage 2013-04-18 05:50:45 UTC
State Changed
From-To: open->feedback

Awaiting maintainers feedback (via the GNATS Auto Assign Tool)
Comment 3 Mark Felder freebsd_committer freebsd_triage 2013-04-18 14:36:19 UTC
I approve this patch
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2013-04-18 15:17:04 UTC
State Changed
From-To: feedback->open

Maintainer approved.
Comment 5 Chris Rees freebsd_committer freebsd_triage 2013-04-18 18:12:31 UTC
Responsible Changed
From-To: freebsd-ports-bugs->crees

I'll take it.
Comment 6 Chris Rees freebsd_committer freebsd_triage 2013-04-18 19:31:23 UTC
State Changed
From-To: open->feedback

I've added some style fixes too; 
http://www.bayofrum.net/~crees/patches/177944.diff William, I used sed 
-n to replace a call to grep, please would you test it still works? 
Mark, is this OK?
Comment 7 will 2013-04-18 21:28:43 UTC
There's more than one host entry in the config file, in different
sections. sabnzbd seems to guarantee that that one that indicates the
address we're listening on comes first.

Your sed command pulls out each host entry (in my case, two).  You can
either head out the first entry, or revert to the grep.

Another alternative would be to use python to parse the ini file and
pull out the *exact* entry using ConfigParser. This seems like a fairly
heavyweight solution though.
Comment 8 Mark Felder freebsd_committer freebsd_triage 2013-04-23 16:21:35 UTC
Patch applies cleanly, but it appears to be having a problem:

Stopping sabnzbd
+ [ -f /home/feld/.sabnzbd/sabnzbd.ini ]
+ grep ^api_key /home/feld/.sabnzbd/sabnzbd.ini
+ tr -d ' _'
+ apikey=apikey=b89c955f5a729f32ae83a9d958513522
+ grep -m1 -E '^host\ =\ [0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}'  
/home/feld/.sabnzbd/sabnzbd.ini
+ tr -dc '[0-9].'
+ host=127.0.0.1
+ sed -nE 's,^host[[:space:]]*=[[:space:]]*(.+),\1,p' /sabnzbd.ini
sed: /sabnzbd.ini: No such file or directory
+ host=''
+ [ = 0.0.0.0 ]
[: =: unexpected operator
+ grep -m1 ^port /home/feld/.sabnzbd/sabnzbd.ini
+ tr -dc '[0-9]'
+ port=8080
+ fetch -o /dev/null  
'http://:8080/api?mode=shutdown&apikey=b89c955f5a729f32ae83a9d958513522'
+ _return=1
+ [ 1 -ne 0 ]
+ [ -z '' ]
+ return 1
+ return 1


It seems to be unable to find my sabnzbd.ini at one point even though  
sabnzbd_conf_dir is set. It then cannot find my IP/hostname as a result,  
and fails to stop the service.

I'll try to dig into this.
Comment 9 Mark Felder freebsd_committer freebsd_triage 2013-04-23 16:40:49 UTC
Aha!

changed $sabnzd_conf_dir to the correctly spelled and styled  
${sabnzbd_conf_dir} which fixed the first problem. Then I noticed when  
testing that sed command it was giving multiple results. This is because  
when you add news servers they appear in a section further down in the  
config and also start with "host = " which the regex was matching. I added  
awk '{print $1}' to only grab the first value. That seems to be working.  
If there's a more lightweight solution (cut?) that might be better.


host=`sed -nE 's,^host[[:space:]]*=[[:space:]]*(.+),\1,p' \
     ${sabnzd_conf_dir}/sabnzbd.ini | awk '{print $1}'`


See attached
Comment 10 Mark Felder freebsd_committer freebsd_triage 2013-04-23 16:44:40 UTC
My final suggestion -- grep -v 'grep' is not necessary. you can just do  
grep [p]ython...
Comment 11 will 2013-04-23 17:09:32 UTC
Did you not see my last mail where I pointed out the host problem? ;-)

Anyway, awk is definitely the better choice here, because cut isn't the 
best with whitespace delimiters.

Looks good to me.

> Mark Felder <mailto:feld@feld.me>
> April 23, 2013 11:40 AM
> Aha!
>
> changed $sabnzd_conf_dir to the correctly spelled and styled 
> ${sabnzbd_conf_dir} which fixed the first problem. Then I noticed when 
> testing that sed command it was giving multiple results. This is 
> because when you add news servers they appear in a section further 
> down in the config and also start with "host = " which the regex was 
> matching. I added awk '{print $1}' to only grab the first value. That 
> seems to be working. If there's a more lightweight solution (cut?) 
> that might be better.
>
>
> host=`sed -nE 's,^host[[:space:]]*=[[:space:]]*(.+),\1,p' \
>     ${sabnzd_conf_dir}/sabnzbd.ini | awk '{print $1}'`
>
>
> See attached
Comment 12 Chris Rees freebsd_committer freebsd_triage 2013-12-04 20:16:28 UTC
Responsible Changed
From-To: crees->feld

Oh dear, I've neglected this... Please would you check this is still 
relevant and commit???
Comment 13 dfilter service freebsd_committer freebsd_triage 2013-12-05 19:32:56 UTC
Author: feld
Date: Thu Dec  5 19:32:43 2013
New Revision: 335681
URL: http://svnweb.freebsd.org/changeset/ports/335681

Log:
  Redesigned rc script to use a pidfile instead of attempting to use web API
  which cannot stop the server in some configurations.
  
  I've confirmed in the python codepath that both the web API shutdown and
  the signal handling both call save_state() so it is safe to shutdown
  this way.
  
  PR:		ports/177944
  Approved by:	crees (mentor)

Modified:
  head/news/sabnzbdplus/Makefile
  head/news/sabnzbdplus/files/sabnzbd.in

Modified: head/news/sabnzbdplus/Makefile
==============================================================================
--- head/news/sabnzbdplus/Makefile	Thu Dec  5 19:13:14 2013	(r335680)
+++ head/news/sabnzbdplus/Makefile	Thu Dec  5 19:32:43 2013	(r335681)
@@ -2,6 +2,7 @@
 
 PORTNAME=	sabnzbdplus
 PORTVERSION=	0.7.16
+PORTREVISION=	1
 CATEGORIES=	news
 MASTER_SITES=	SF/${PORTNAME}/${PORTNAME}/${PORTVERSION}
 DISTNAME=	SABnzbd-${PORTVERSION}-src
@@ -70,10 +71,9 @@ USES=		gettext
 NO_BUILD=	yes
 WRKSRC=		${WRKDIR}/SABnzbd-${PORTVERSION}
 PLIST_SUB=	PORTNAME=${PORTNAME}
-SUB_LIST+=	PORTNAME=${PORTNAME}
+SUB_LIST+=	PORTNAME=${PORTNAME} PYTHON_CMD=${PYTHON_CMD}
 SUB_FILES=	pkg-message
 USE_RC_SUBR=	sabnzbd
-CONFLICTS_INSTALL=	sabzndb-0.*
 
 PORTDOCS=	ABOUT.txt \
 		CHANGELOG.txt \

Modified: head/news/sabnzbdplus/files/sabnzbd.in
==============================================================================
--- head/news/sabnzbdplus/files/sabnzbd.in	Thu Dec  5 19:13:14 2013	(r335680)
+++ head/news/sabnzbdplus/files/sabnzbd.in	Thu Dec  5 19:32:43 2013	(r335681)
@@ -34,68 +34,22 @@ load_rc_config ${name}
 : ${sabnzbd_group:=_sabnzbd}
 : ${sabnzbd_conf_dir="%%PREFIX%%/sabnzbd"}
 
-required_dirs=${sabnzbd_conf_dir}
+pidfile=/var/run/sabnzbd/sabnzbd-$(grep -m1 ^port ${sabnzbd_conf_dir}/sabnzbd.ini | tr -dc '[0-9]').pid
 
-start_cmd="${name}_start"
-status_cmd="${name}_status"
-stop_cmd="${name}_stop"
 start_precmd="${name}_prestart"
+extra_commands="status"
+command_interpreter="%%PYTHON_CMD%%"
+command="%%PREFIX%%/bin/SABnzbd.py"
+command_args="--daemon -f ${sabnzbd_conf_dir}/sabnzbd.ini --pid ${pidfile%/*}"
 
 sabnzbd_prestart()
 {
-	PATH=${PATH}:%%PREFIX%%/bin:%%PREFIX%%/sbin
-	if [ ! -f "${required_dirs}" -a ! -d "${required_dirs}" -a ! -L "${required_dirs}" ]; then
-		install -d -o ${sabnzbd_user} -g ${sabnzbd_group} ${required_dirs}
-	fi
-}
-
-sabnzbd_start()
-{
-	if [ ! -f "${sabnzbd_pid}" ]; then
-		su -m ${sabnzbd_user} -c "%%PREFIX%%/bin/SABnzbd.py --daemon -f ${sabnzbd_conf_dir}/sabnzbd.ini"
-		echo "Starting ${name}."
-	else
-		GETPROCESSPID=`/bin/ps -auxw | /usr/bin/awk '/SABnzbd.py/ && !/awk/ && !/sh/ {print $2}'`
-		PIDFROMFILE=`cat ${sabnzbd_pid}`
-		if [ "$GETPROCESSPID" = "$PIDFROMFILE" ]; then
-			echo "${name} already running with PID: ${PIDFROMFILE} ?"
-			echo "Remove ${sabnzbd_pid} manually if needed."
-		else
-			rm -f ${sabnzbd_pid}
-			su -m ${sabnzbd_user} -c "%%PREFIX%%/bin/SABnzbd.py --daemon -f ${sabnzbd_conf_dir}/sabnzbd.ini"
-			echo "Starting ${name}."
+	PATH=${PATH}:/usr/local/bin:/usr/local/sbin
+	for sabdir in ${sabnzbd_conf_dir} ${pidfile%/*}; do
+		if [ ! -d "${sabdir}" ]; then
+			install -d -o ${sabnzbd_user} -g ${sabnzbd_group} ${sabdir}
 		fi
-	fi
-}
-
-# SABnzbd can only be cleanly stopped by calling the http api
-sabnzbd_stop()
-{
-	echo "Stopping $name"
-	if [ -f "${sabnzbd_conf_dir}/sabnzbd.ini" ]; then
-		apikey=`grep ^api_key ${sabnzbd_conf_dir}/sabnzbd.ini | tr -d " _"`
-		host=`grep -m1 -E '^host\ =\ [0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}' ${sabnzbd_conf_dir}/sabnzbd.ini | tr -dc '[0-9].'`
-		if [ ${host} = "0.0.0.0" ] ; then 
-			host="localhost" ;
-		fi
-		port=`grep -m1 ^port ${sabnzbd_conf_dir}/sabnzbd.ini | tr -dc '[0-9]'`
-		fetch -o /dev/null "http://${host}:${port}/api?mode=shutdown&${apikey}" > /dev/null 2>&1
-	else
-		sabnzbd_pid=`ps -U ${sabnzbd_user} | grep "python.*SABnzbd.py.*--daemon" | grep -v 'grep' | awk '{print $1}'`
-	        if [ -n "${sabnzbd_pid}" ]; then
-			kill ${sabnzbd_pid}
-		fi
-	fi
-}
-
-sabnzbd_status()
-{
-	sabnzbd_pid=`ps -U ${sabnzbd_user} | grep "python.*SABnzbd.py.*--daemon" | grep -v 'grep' | awk '{print $1}'`
-	if [ -n "${sabnzbd_pid}" ]; then
-		echo "$name is running as ${sabnzbd_pid}"
-	else
-		echo "$name is not running"
-	fi
+	done
 }
 
 run_rc_command "$1"
_______________________________________________
svn-ports-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-ports-all
To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"
Comment 14 Mark Felder freebsd_committer freebsd_triage 2013-12-05 19:34:31 UTC
State Changed
From-To: feedback->closed

Rewrote rc script to resolve this PR