http://redmine.audacious-media-player.org/http://redmine.audacious-media-player.org/welcome/favicon.ico?15159353402017-03-22T04:23:58ZRedmineAudacious - Feature #713: Add optional "url_helper" script (config file option) for playing streams that need "help" to playhttp://redmine.audacious-media-player.org/issues/713?journal_id=26412017-03-22T04:23:58ZJohn Lindgrenjohn@jlindgren.net
<ul></ul><p>Please use unified diff format. It's not easy to see what you changed without context.</p> Audacious - Feature #713: Add optional "url_helper" script (config file option) for playing streams that need "help" to playhttp://redmine.audacious-media-player.org/issues/713?journal_id=26422017-03-22T04:27:34ZJohn Lindgrenjohn@jlindgren.net
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Rejected</i></li></ul><pre>system ((const char *) str_concat ({url_helper, " ", filename, " ", aud_get_path (AudPath::UserDir)}));
</pre>Hell no. Executing of a system call with unsanitized, unquoted, unescaped user input <strong>screams</strong> security hole. Audacious - Feature #713: Add optional "url_helper" script (config file option) for playing streams that need "help" to playhttp://redmine.audacious-media-player.org/issues/713?journal_id=26432017-03-22T04:34:07ZJim Turnerturnerjw784@yahoo.com
<ul><li><strong>File</strong> <a href="/attachments/640/playlist-files.cc.diff">playlist-files.cc.diff</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/640/playlist-files.cc.diff">playlist-files.cc.diff</a> added</li><li><strong>File</strong> <a href="/attachments/641/playlist-files.cc">playlist-files.cc</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/641/playlist-files.cc">playlist-files.cc</a> added</li></ul><p>Ok. I wasn't sure if it would make sense. Pbm. is that I haven't (yet) attempted to merge your latest "playlist" commits, so I'm diffing against a slightly older Audacious version. I've also attached my latest version, which may help.</p>
<p>Jim</p> Audacious - Feature #713: Add optional "url_helper" script (config file option) for playing streams that need "help" to playhttp://redmine.audacious-media-player.org/issues/713?journal_id=26442017-03-23T06:04:23ZJohn Lindgrenjohn@jlindgren.net
<ul></ul><p>Your second diff got messed up somehow (it's only 106 bytes) but I can see the changes from the full file. Thanks.</p>
<p>My earlier comment was a bit brusque and I apologize for that. I do appreciate the creativity of this solution, but I don't think this change should be included in Audacious. If we were going to implement a URL "translation" feature, I would want to see it done with a proper plugin, not by hacking up libaudcore to execute some Perl script. I think it could be done by adding an isOurPlaylist() function to the PlaylistPlugin class to treat a "special" URL as a playlist, then loading that "playlist" through the normal PlaylistPlugin::load() method. I'm not saying I'd want to maintain such a plugin in-tree, but I would be okay with adding the API to support it.</p> Audacious - Feature #713: Add optional "url_helper" script (config file option) for playing streams that need "help" to playhttp://redmine.audacious-media-player.org/issues/713?journal_id=26462017-03-23T13:45:04ZJim Turnerturnerjw784@yahoo.com
<ul></ul><p>Thanks John for the suggestion on how to better do this. Working on Audacious is always a big learning experience for me as I only know the "tip of the iceberg" on this thing, so everything I do is a trial and error approach for me, so I'm always open to advice from the guru. I should pbly pay a lot more attention to security as well. As a fellow techie, no pbm. - I'm pretty used to both getting and giving brusque, to the point comments. You're pretty tame compared to some of the things Linus Torvalds says among kernel devs!</p>
<p>Cheers,</p>
<p>Jim</p>