New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Newline-Characters in ID3-Tag-Fields are not escaped #881

Open
lukasvogel opened this Issue Oct 23, 2014 · 1 comment

Comments

3 participants
@lukasvogel

lukasvogel commented Oct 23, 2014

When playing a song which has a ID3-Tag-Field with multiple files (i.E. the one that produced this log: http://pastebin.com/CZ8UF6Z6), the newlines won't get escaped. This results in clients not accepting the sent message is it is not conforming with the protocol.

@adamcik

This comment has been minimized.

Member

adamcik commented Dec 14, 2014

Comments should not be exposed in the track metadata for the MPD protocol. There is a new readcomments command which is the only place "comments" are exposed in the latest MPD. However, these comments are things like ASIN, not the comments we are fetching.

So followup needs to be:

  • Remove the comments from the MPD translator
  • Add code to either warn about or escape low ASCII chars to make our protocol impl. safer. (note that MPD responses are somewhat under specified in their docs, so we just have to play it safe)

adamcik added a commit to adamcik/mopidy that referenced this issue Dec 14, 2014

mpd: Remove "Comment" tag type from translator output.
Newer versions of the protocol have removed this tag, so we should as well.
This also works around the issue of mopidy#881 which was breaking things with
newlines in comment fields.

The readcomments command seems to replace this, but it seems to only care about
specific extra tagtypes, not the general comment tag we normally collect when
scanning things.

adamcik added a commit to adamcik/mopidy that referenced this issue Dec 14, 2014

mpd: Remove newline escaping code.
This was added for mopidy#881, where the correct fix turned out to be to remove
comments from the responses. We should still add some sanity checks for
verifying that our responses at the very least only contain printable chars.

adamcik added a commit that referenced this issue Dec 16, 2014

mpd: Remove "Comment" tag type from translator output.
Newer versions of the protocol have removed this tag, so we should as well.
This also works around the issue of #881 which was breaking things with
newlines in comment fields.

The readcomments command seems to replace this, but it seems to only care about
specific extra tagtypes, not the general comment tag we normally collect when
scanning things.

(cherry picked from commit 08a8d5c)

@jodal jodal modified the milestone: v0.19.5 - Bug fixes Dec 23, 2014

teq0 added a commit to teq0/mopidy that referenced this issue Feb 3, 2015

mpd: Remove newline escaping code.
This was added for mopidy#881, where the correct fix turned out to be to remove
comments from the responses. We should still add some sanity checks for
verifying that our responses at the very least only contain printable chars.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment