Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 598466

Summary: net-nntp/sabnzbd Change to init script causes potential issue
Product: Gentoo Linux Reporter: eponymous <the.epon>
Component: Current packagesAssignee: Joe Kappus <joe>
Status: RESOLVED WONTFIX    
Severity: normal CC: gentoo, jsbronder, jstein, proxy-maint, the.epon
Priority: Normal    
Version: unspecified   
Hardware: All   
OS: Linux   
Whiteboard:
Package list:
Runtime testing required: ---

Description eponymous 2016-10-29 18:39:38 UTC
Just saw the changes here:

author	Michał Kępień <github@kempniu.pl>	2016-10-05 09:08:55 (GMT)
committer: Justin Bronder <jsbronder@gentoo.org>	2016-10-28 12:57:14 (GMT)
commit	975400d5765f7405f611ddf66a3ad717cb21a203 (patch)
tree	63af103d41e9e10781cb92af4b7ed040b4052eff /net-nntp/sabnzbd

This change is dangerous:

get_var() {
-    echo $(sed -n \
-        '/^\[misc]/,/^'$1'/ s/^'$1' = \([[:alnum:].]\+\)[\r|\n|\r\n]*$/\1/p' \
-        "${SABNZBD_CONFIGFILE}")
+	grep -P -o -m 1 "(?<=^${1} = ).*" "${SABNZBD_CONFIGFILE}" || echo 0
 }

The script previously was only looking for values (deliberately) in [misc] section which allowed for that [misc] section to appear anywhere in the file.

The new code is now wrongly assuming the [misc] section will always be the first section in the file which may change in the future. This would break horribly if it did. The key "port" appears multiple times in the file for example.
Comment 1 Michał Kępień 2016-10-30 20:32:12 UTC
The issue you describe only pertains to "host" and "port" variables, so
if by "break horribly", you mean "fall back to SIGTERM and do what it is
supposed to do", then yeah, I guess it will "break horribly" in such a
case :)

As I wrote in the line comment at GitHub [1], SABnzbd+ does not reorder
sections when writing the configuration file.  As the [misc] section is
the first section in both the configuration file used by the Gentoo
ebuild and the one generated by SABnzbd+ itself in case no configuration
file is present, the situation you describe would only happen when the
user rearranges sections by hand (e.g. using an editor).  Sure, it is
possible, but how many people have actually done that?  Again, even if
it happens, the script will still do what it is supposed to.

This is obviously a maintainer's call to make and I will not blindly
defend my position.  Quite frankly, I do not care enough, I just happen
to like keeping things simple when possible.  Though if get_var() ends
up using sed again, it would be nice if the other issues [2] got ironed
out.

[1] https://github.com/gentoo/gentoo/pull/2477#discussion_r82557099
[2] https://github.com/gentoo/gentoo/pull/2477#discussion_r82556994
Comment 2 Justin Bronder (RETIRED) gentoo-dev 2016-11-02 01:20:01 UTC
Personally, and I very well could be wrong, I'm going to stick with Michał's changes.  However, I'd happily accept a patch that also make sure to check the right section is being parsed just in case the user decided to move things around assuming it does still address the other bugs brought up on github.
Comment 3 Oleh 2016-11-20 08:40:29 UTC
# rc-service sabnzbd start
sabnzbd            | * Starting SABnzbd ...
sabnzbd            | * start-stop-daemon: failed to start `/usr/share/sabnzbd/SABnzbd.py'                                                                                             [ !! ]d            |
sabnzbd            | * ERROR: sabnzbd failed to start

test changes before merging into tree.
Comment 4 Michał Kępień 2016-11-20 11:40:54 UTC
Whatever the issue that you are affected by is, I fail to see its
connection with the issue discussed in this bug (i.e. what get_var()
does - note that get_var() is not used when SABnzbd+ is started).  Could
you please open another bug report with an error log attached and CC me?
Comment 6 Sébastien P. 2016-11-20 22:04:42 UTC
#600356
Comment 7 Joe Kappus 2022-11-30 00:10:20 UTC
Greetings from 6 years in the future! This format hasn't changed even after a refactor to python3 and no reports have been about issues caused since this bug was raised. All the other things discussed in comments are python2 issues that are obsoleted by sabnzbd moving to py3. 

I realize you may not like this resolution but history has shown it's not currently a problem. IF that ever changes, file a new bug and I'll revisit this.