Opened 5 years ago

Closed 5 years ago

#12169 closed Patch - Bug Fix (Invalid)

Patch: Allow mythfilldatabase logging when full pathname is used in mythtv-setup

Reported by: Biill Meek <keemllib@…> Owned by: Raymond Wagner
Priority: minor Milestone: unknown
Component: MythTV - Housekeeper Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

Based on comments in the -users list, a full pathname is sometimes used in the "Guide data program" setting (MythFillDatabasePath?.)

For example: http://www.gossamer-threads.com/lists/mythtv/users/570672#570672

Currently, if anything other than "mythfilldatabase" is used in that setting, logging parameters passed by the backend won't be used by MFDB and no MFDB logs will be made.

Tested against: v0.28-pre-1572-g220ee25-dirty

Attachments (1)

mfdb-full-pathname.patch.v0 (699 bytes) - added by Bill Meek <keemllib@…> 5 years ago.

Download all attachments as: .zip

Change History (4)

Changed 5 years ago by Bill Meek <keemllib@…>

Attachment: mfdb-full-pathname.patch.v0 added

comment:1 Changed 5 years ago by Raymond Wagner

MythTV will never itself set that argument to anything but mythfilldatabase, and it cannot assume the behavior of any executable other than the ones it installed into its own prefix. I don't see how this is anything but a user configuration error. If we are going to change it to help prevent user misconfiguration, I would rather see an explicit checkbox to "use default command" than a more permissive string check.

comment:2 Changed 5 years ago by sphery

For MythTV 0.19, the default value of "mythfilldatabse Path" was changed from "mythfilldatabase" to gContext->GetInstallPrefix?() + "/bin/mythfilldatabase" because someone complained that with just "mythfilldatabase" (as in 0.18), an evil user on the local system could use a PATH-based attack to execute a script with the permissions of the user running the backend by placing a similarly-named script in an earlier-in-the-PATH directory. The default stayed this way in 0.20, but 0.21 was fixed to use the current approach, which uses "mythfilldatabase" as the default value for "mythfilldatabase Path".

With the current approach, "mythfilldatabase"--only and exactly that string, with no path--is not a command. It's a sentinel value that says to run mythfilldatabase from the program's INSTALL_PREFIX (which /is/ supplied as part of the command to run mythfilldatabase if the "mythfilldatabase Path" is set to "mythfilldatabase" to prevent PATH-based attacks) and to assume it is the packaged-with-MythTV-source version of mythfilldatabase and that it will use MythTV's logging options, so it should be passed the same logging options as mythbackend (unless other logging options were explicitly specified).

If any other string is provided for "mythfilldatabase Path" (even /usr/bin/mythfilldatabase or /usr/local/bin/mythfilldatabase), it is assumed to be a user-override of defaults and the command is executed as supplied (so users can provide path-less commands and rely on environment PATH settings or can provide absolute paths to the commands and ensure a program at the specified location is executed).

Whether all programs called mythfilldatabase (regardless of path) should be considered to be the "canonical" mythfilldatabase application/should be given mythbackend's logging options is a question I'll leave for someone else to decide. The current design is simple, but provides flexibility for user-/distro-provided scripts.

That said, I do not think a simple endsWith("mythfilldatabase") check for whether to pass mythbackend logging options is appropriate (i.e. users may make "mymythfilldatabase" or "/usr/local/bin/custommythfilldatabase" which probably should be considered "not the default"). Even an endsWith("/mythfilldatabase") is probably not appropriate as users may have created a custom script at /home/mythtv/mythfilldatabase or may even have MythTV installed to /usr , but created a /usr/local/bin/mythfilldatabase script for providing guide data/managing guide data retrieval.

Rather than a "use default command" setting (or even a UI element that resets the existing "mythfilldatabase Path" to its default value), it would be nice to provide a mechanism within the settings editor that allows any setting to be reset to its default value.

And remember that without any changes, the (presumably small) percentage of users who initialized their MythTV database with MythTV 0.18 or 0.19 can reset their "mythfilldatabase Path" setting to "mythfilldatabase" (removing any path component) if they are using direct calls to the canonical mythfilldatabase application so that logging is performed according to the options specified to mythbackend. Alternatively, custom logging options can be specified in the "Guide data arguments" setting so that mythfilldatabase always uses the same logging options, regardless of those specified for mythbackend.

comment:3 Changed 5 years ago by paulh

Resolution: Invalid
Status: newclosed

This seems to be working as intended. See comment 2.

Note: See TracTickets for help on using tickets.