"400 Bad Request" when playlist is empty

[maartenlangeveld]

Hello Arthur,

I get a "400 Bad Request" when I select "Now Playing" page and playlist is empty.
This behaviour started after my Raspberry Pi distribution had upgraded from php7.0 to php7.3.

When playlist is not empty, I can navigate to "Now Playing" page as before.

May be php7.3 is the cuase of this issue?

"http://dietpi/ompd/message.php?message=%5Bb%5DPlaylist%20is%20empty%5B%2Fb%5D%5Bbr%5D%5Bbr%5D%09%5Burl%3Dindex.php%5DAdd%5B%2Furl%5D%20some%20music!&type=warning&menu=playlist&skin=ompd_dark&username=ompd&sign=sneBt2SRtvyaSsrZPP6QATaZiZPaDe7JfA81_a8e&timestamp=5cdef9a1"

Thanks for your great O!MPD. I really like it.

Maarten

[ArturSierzant]

Hello Maarten,
thanks for feedback. I've just updated my php from 7.0 to 7.3 (I use raspbian) and I don't have any issue with empty playlist. Maybe it's a matter of web server? I use Apache, what do you use?

[maartenlangeveld]

Hi Arthur,

Thanks, It's lighttpd. With new version came a new config file. Put back in the old config file and now everything is ok.

Kind regards, Maarten

New version lighttpd added following lines to lighttpd.conf. When I disable "url-ctrls-reject" "Now Playing" page loads again when playlist is empty.

# strict parsing and normalization of URL for consistency and security
# https://redmine.lighttpd.net/projects/lighttpd/wiki/Server_http-parseoptsDetails
# (might need to explicitly set "url-path-2f-decode" = "disable"
#  if a specific application is encoding URLs inside url-path)
server.http-parseopts = (
  "header-strict"           => "enable",# default
  "host-strict"             => "enable",# default
  "host-normalize"          => "enable",# default
  "url-normalize-unreserved"=> "enable",# recommended highly
  "url-normalize-required"  => "enable",# recommended
 #"url-ctrls-reject"        => "enable",# recommended -->Disabled. breaks O!MPD 'Now Playing' when playlist is emtpy.
  "url-path-2f-decode"      => "enable",# recommended highly (unless breaks app)
 #"url-path-2f-reject"      => "enable",
  "url-path-dotseg-remove"  => "enable",# recommended highly (unless breaks app)
 #"url-path-dotseg-reject"  => "enable",
 #"url-query-20-plus"       => "enable",# consistency in query string
)

[ArturSierzant]

After quick search I found that url-ctrls-reject means 'reject any percent-encoded control chars'. I'll try to take a look at this.

[maartenlangeveld]

Hi Arthur,

Thanks ! Have a good Sunday.

[ArturSierzant]

It should work now - if not, please let me know.

[maartenlangeveld]

Hi Arthur,

Thanks, works now with url-ctrls-reject enabled !

Hereby I inform @MichaIng and @Fourdee of DietPi.

Regards, Maarten

[MichaIng]

@maartenlangeveld @ArturSierzant
Many thanks for investigating and solving this. This new default Lighttpd config (Debian+Raspbian Buster) indeed seems to break some applications, as also the config comments state.
However as they are security hardening directives at least it might be worth to check if it's possible to change the related script(s) in O!MPD to not use e.g percent-encoded control chars in this case.

Shall we open a new request issue about that?

[ArturSierzant]

In this case problem was in string passed to url - this string contained a tab char, which made lighttpd throw 400 Bad Request. I fixed the function that creates this string, so it should also work in other cases. So please hold on with new request until you find new problem. In the meantime I'll switch to lighttpd (instead of Apache) and try to find similar issues.

Regards,
Artur

[MichaIng]

@ArturSierzant
Perfect, so we just need to use the current O!MPD version, right?

General question since the last release was 2018, the master branch can be considered as stable, right?

[ArturSierzant]

In master branch I put new features - some of them are finished, some are in development. In that sense this branch can't be considered as stable. On the other hand, it contains all new fixes and I don't put there any experimental features.

Latest stable (i.e. with all features finished) version is available on web page (BTW it's 1.04, while DietPi has 1.03). I hope the next version (1.05) will be ready in a few weeks.

[MichaIng]

@ArturSierzant
Okay thanks for the info.

BTW it's 1.04, while DietPi has 1.03

We pinned O!MPD version a while ago due to some observed database update issue, the same for myMPD. So must have been an issue with MPD itself. Wanted to retest with this the current versions anyway.

Just checked the fix you did, nothing we can apply manually. I think we will go with master for now, to avoid changing Lighttpd configs back and forth. After v1.05 release we will then switch to the latest release: https://api.github.com/repos/ArturSierzant/OMPD/releases/latest

[ArturSierzant]

@MichaIng
I've been using DietPi (BTW - great work!) for a while to find out if there are any other issues with O!MPD. Here are my notes and suggestion:

1. Add server.stream-response-body = 1 to lighttpd.conf: this will allow lighttpd to stream response body to client instead of bufferring entire response (default behaviour). In O!MPD I often use @ob_flush(); flush(); and default setting for server.stream-response-body prevents flushing to work correctly - it's important especially during update database when it can cause strange behaviour like running update script for several times in a row.

2. O!MPD uses python to play audio from YouTube movies and to get data from Tidal. For this to work correctly, module requests is needed, but it's not present in default DietPi configuration. Simplest way to add it is sudo apt-get install python-requests - but it would be great to add this module during installing O!MPD.

3. Playing audio from YouTube also requires youtube-dl - it's the python script to extract data from YouTube. Installation instructions are available here: https://github.com/ytdl-org/youtube-dl#installation. This could also be added during installation of O!MPD.

4. To play tracks from Tidal, mpd >= 0.21 is required. Please consider updating mpd in DietPi.

5. After installing O!MPD, link http://[device_ip_address]/ompd is not working. It was reported in How to start on DietPi  #88 and then I confirmed this myself. But link http://[device_ip_address]/ompd/index.php works OK. What's interesting, once I opened this address, http://[device_ip_address]/ompd started to work. I don't know what is the reason of this behaviour. Anyway, I think that updating this link in DietPi documentation could be helpful.

That's it. I hope you'll read this and it'll help you in making DietPi even better.

PS
I'm not opening a new issue on DietPi github page, but if it is more convenient for you, I of course will do it.

Best regards,
Artur

[MichaIng]

@ArturSierzant
Many thanks for your suggestions.

1. Okay I will add this as ompd sub dir config. I guess with Apache2 and Nginx there is some similar directive required?
   EDIT: Indeed adding this Lighttpd config resolves the issue (database update stucks in loop, although just browser-wise, cancelling load shows the database has been successfully updated) that was the reason to stick with an older version of O!MPD: DietPi-Software | O!MPD/myMPD: Database update fails on current upstream versions MichaIng/DietPi#2156 (comment)
   Testing now with Apache2 and Nginx.

   • Same issue with default Nginx config. Could not find a related directive yet. fastcgi_param PHP_ADMIN_VALUE "output_buffering=Off"; and/or fastcgi_request_buffering off; do not help.
   • Apache works well.
   • So we need to find a fix for Nginx.

2. Not sure if we should pre-install it since some users might not require. But either we could prompt an interactive choice or we add the info to our online docs.

3. Same as 3.

4. Jep makes sense to upgrade our shipped binaries in general.

5. Will check this, AFAIK this worked fine in my case. With Lighttpd as browser, right?
   EDIT: Hmm works well here right from the beginning. index.php is also the default first index file, which should assure this. Not sure. Will try on some other devices (this was testing VM) by times. Perhaps it's also some browser caching issue or such. Bad requests can sometimes be solved by ctrl+f5 (clear cache and reload page).

I'm not opening a new issue on DietPi github page, but if it is more convenient for you, I of course will do it.

Perhaps I will do it myself or simply a PR to implement the above suggestions.
EDIT: Ah can all be done together with: MichaIng/DietPi#2816 and MichaIng/DietPi#2156

[MichaIng]

@ArturSierzant
In case sorry for asking unrelated questions here but we already went off-topic anyway 😄:

• Is it safe to migrate old config files to new O!MPD versions?
• Currently we simply purge the old instance WITH config file, but user settings/changes are lost then. So would be beneficial to backup the old config file and automatically restore it to the new instance.
• But if e.g. deprecated settings cause issues, then this is no option. At best those are even translated or removed automatically on first check/access to new instance? 😉

[ArturSierzant]

I think the best solution is putting DietPi-specific settings in file include\config.local.inc.php. Settings from this file override ones from config.inc.php. So simplest file config.local.inc.php could look like this:

<?php
$cfg['mysqli_host']                 = '127.0.0.1';
$cfg['mysqli_db']                   = 'ompd';
$cfg['mysqli_user']                 = 'ompd';
$cfg['mysqli_password']             = 'dietpi';
$cfg['mysqli_port']                 = '3306';
$cfg['mysqli_socket']               = '';
$cfg['mysqli_auto_create_db']       = true;

$cfg['media_dir']                   = '/mnt/dietpi_userdata/Music';
?>

I'm not sure if I remember correctly values above (username, pass, etc.).

Rest of settings is taken from config.inc.php, which usually changes from version to version, i.e. old settings remain, but some new ones are added. So restoring it in new version will not work (user might lose some of new features from new version of O!MPD).

Any changes made by user would then have to be done in config.local.inc.php. So if for example user would like to change $cfg['multidisk_indicator'], he would have to add the following to config.local.inc.php:

unset($cfg['multidisk_indicator']);

$cfg['multidisk_indicator'][] = "Disc ";
$cfg['multidisk_indicator'][] = "Disk ";
$cfg['multidisk_indicator'][] = " (bonus dis";
$cfg['multidisk_indicator'][] = "CD#";
$cfg['multidisk_indicator'][] = " CD";
$cfg['multidisk_indicator'][] = " Vol.";
$cfg['multidisk_indicator'][] = " '";

and modify (add/remove/change) desired values. Then DietPi could copy config.local.inc.php file and restore it in new version as the file containing user configuration.

[maartenlangeveld]

@MichaIng

$cfg['mysqli_password'] = 'dietpi';

At my install password = ompd, do not remember if this default dietpi or that I have changed password for some reason.

[MichaIng]

@ArturSierzant
Indeed using the override config totally makes sense. I also just checked that editing the settings via web UI then edits the local override. That also makes reinstalls easier by just copying the override config and if it exists, we can assume that it is a reinstall and we should not touch the config at all.

Found that database is automatically created and migrated. So we can leave the old one in place and on fresh installs only create the database user 👍.

Now things are developing as I like it 🙂.
Btw I also added YouTube + Tidal support as you suggested: MichaIng/DietPi#2884

• Since Python is pulled as dependency for MPD (libsmbclient actually), the overhead is minimal.

Little error message from Lighttpd, when updating the database, although I can see any effect on functionality:

root@VM-Stretch:/var/www/ompd/include# cat /var/log/lighttpd/error.log
2019-06-04 15:31:45: (log.c.217) server started
2019-06-04 15:32:17: (mod_fastcgi.c.2543) FastCGI-stderr: PHP message: PHP Warning:  Division by zero in /var/www/ompd/update.php on line 715PHP message: PHP Warning:  Division by zero in /var/www/ompd/update.php on line 715

And another little enhancement:

• I switched to MySQL socket connection, which should be generally faster if webserver and MySQL/MariaDB are on the same machine:

$cfg['mysqli_port']                 = '';
$cfg['mysqli_socket']               = '/run/mysqld/mysqld.sock';

• Works well, however is it intended to make port empty string to force socket connection or is a value, e.g. "0" expected or can it even be left unchanged, e.g. if socket is set and reachable, it is preferred automatically? Sorry in case this is a question of the PHP mysqli module and not of O!MPD, I don't know too much details about how this works 😉.

---

@maartenlangeveld
The password is the global software password that you chose via dietpi.txt or after logging in the first time. Default is "dietpi" if one does do not change it.

[ArturSierzant]

Great to read you added YouTube and Tidal support!

Unfortunately I can't help you with socket connection - I never used it, it is leftover after Netjukebox,

And this PHP warning - indeed it's not harmful, but I'll take a closer look at this (strange, because I thought, it's already fixed...)

[MichaIng]

@ArturSierzant
No problem about socket, it works just well, according to my tests and is enabled by default MariaDB-side. As mentioned for performance and as well security reasons, socket access should be generally preferred, regardless of which type of server daemon actually. But server and client need to be on the same machine of course. In this case TCP accessibility can be disabled (security aspect), e.g. on MariaDB:

[mysqld]
skip-networking

Okay so last ToDo my end:

• Find a solution for database update loop on Nginx, so an equivalent to server.stream-response-body = 1 on Lighttpd.
• According to ompd.pl, you didn't test O!MPD on Nginx, however beside this one issue, it is working very well 🙂.

[ArturSierzant]

@MichaIng
I think, I found the solution for buffering issue in nginx. I added it in commit 5806095. It turned out that buffering can be switched off by modifying the header: header('X-Accel-Buffering: no');
Please check, if this solves nginx issues.

[MichaIng]

@ArturSierzant
Bingo, that solved the database update loop on Nginx, nice find! Is server.stream-response-body = 1 on Lighttpd then has still an effect or is disabled buffering == streaming response?

[ArturSierzant]

I've just checked - header('X-Accel-Buffering: no'); is not working for lighttpd, it still requires server.stream-response-body = 1.

[MichaIng]

@ArturSierzant
Just to assure, with 3346ee1 python-requests became obsolete, right? We implement youtube-dl as dedicated software option, setup to be called with Python 3 and I'm not keen to keep Python 2 dependencies anymore 😉.

[ArturSierzant]

Yes, that's right - python is no longer used in O!MPD to serve Tidal requests. However youtube-dl is still used to get YT stream url. Path to youtube-dl is defined in $cfg['youtube-dl_path'] in file include/config.inc.php.

[MichaIng]

Great. Yes, youtube-dl will still be installed together with O!MPD, but called with Python 3.