Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 598466 - net-nntp/sabnzbd Change to init script causes potential issue
Summary: net-nntp/sabnzbd Change to init script causes potential issue
Status: UNCONFIRMED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Thomas Deutschmann
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-29 18:39 UTC by eponymous
Modified: 2020-02-10 11:10 UTC (History)
4 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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