MPD protocol: "noidle" command returns "OK" even when not in idle

[geneticdrift]

owntone 28.4
Debian 4.19.282-1

MPD ignores "noidle" commands unless the client connection is already in "idle" state.
Owntone responds with "OK" to "noidle" even when the connection is not in "idle" state.

To reproduce:

# this is owntone response to noidle
echo "noidle" | nc -N 192.168.122.149 6603
OK MPD 0.20.0
OK

# this is MPD response to noidle
echo "noidle" | nc -N localhost 6601
OK MPD 0.24.0

This may cause MPD clients to not be able to keep track of the response to request sequence.
A client needs to send "noidle" when the last command it sent was "idle" to interrupt it and send another command.
But it is possible that the client connection in MPD has already sent a response to the last "idle" that the client has not yet received. So MPD must ignore the noidle command unless the client connection is still in idle state.

[ejurgensen]

The maintainer of the mpd part of owntone isn't active any more. I can make a fix if it is a simple one, but I'm not myself well acquiented with the protocol, and I didn't completely understand your description. Could you spell out for me what it is owntone should or shouldn't do?

[geneticdrift]

Could you spell out for me what it is owntone should or shouldn't do?

When noidle is received and the client connection is not in idle state then it should just ignore the noidle command, not output OK or anything else.

The fix for this is simple, but I found other problems with the MPD protocol implementation in owntone mainly in mpd_read_cb handling of multiple command lines from the input buffer.

For example it breaks from the loop on COMMAND_LIST_END when there are potentially more commands still in the buffer. It also doesn't reset the idle_cmd flag for each new command, and it breaks out of the loop on malformed command potetially leaving unhandled commands in the input buffer.

If you like I can try fixing it and making a pull request.

[ejurgensen]

That would be awesome, sounds like you know this stuff

[grobian]

isn't this just an else if (ctx->is_idle) instead of else in mpd_command_noidle to fix this?

[ejurgensen]

Fixed in PR #1794