Discussion:
[PATCH 1/2] id3: fix id3_skiplen() for UTF-16 and UTF-16BE
Gregory Petrosyan
2011-04-09 17:57:50 UTC
Permalink
Signed-off-by: Gregory Petrosyan <***@gmail.com>
---
id3.c | 48 +++++++++++++++++++++++++++---------------------
1 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/id3.c b/id3.c
index 52e0604..5609cd9 100644
--- a/id3.c
+++ b/id3.c
@@ -19,6 +19,15 @@
#include <strings.h>
#include <limits.h>

+enum {
+ ID3_ENCODING_ISO_8859_1 = 0x00,
+ ID3_ENCODING_UTF_16 = 0x01,
+ ID3_ENCODING_UTF_16_BE = 0x02,
+ ID3_ENCODING_UTF_8 = 0x03,
+
+ ID3_ENCODING_MAX = 0x03
+};
+
/*
* position:
*
@@ -638,12 +647,12 @@ static char *decode_str(const char *buf, int len, int encoding)
char *in, *out = NULL;

switch (encoding) {
- case 0x00: /* ISO-8859-1 */
+ case ID3_ENCODING_ISO_8859_1:
in = xstrndup(buf, len);
utf8_encode(in, id3_default_charset, &out);
free(in);
break;
- case 0x03: /* UTF-8 */
+ case ID3_ENCODING_UTF_8:
in = xstrndup(buf, len);
if (u_is_valid(in)) {
out = in;
@@ -652,8 +661,8 @@ static char *decode_str(const char *buf, int len, int encoding)
free(in);
}
break;
- case 0x01: /* UTF-16 */
- case 0x02: /* UTF-16BE */
+ case ID3_ENCODING_UTF_16:
+ case ID3_ENCODING_UTF_16_BE:
out = utf16_to_utf8((const unsigned char *)buf, len);
break;
}
@@ -705,25 +714,22 @@ static void decode_normal(struct id3tag *id3, const char *buf, int len, int enco
add_v2(id3, key, out);
}

-static int enc_is_utf16(int encoding)
-{
- return encoding == 0x01 || encoding == 0x02;
-}
-
-static size_t id3_skiplen(const char *s, int encoding)
+static size_t id3_skiplen(const char *buf, size_t len, int encoding)
{
- size_t ret = strlen(s) + 1;
+ if (encoding == ID3_ENCODING_ISO_8859_1 || encoding == ID3_ENCODING_UTF_8) {
+ return strlen(buf) + 1;
+ } else {
+ int i = 0;
+ while (i + 1 < len) {
+ if (buf[i] == '\0' && buf[i + 1] == '\0')
+ return i + 2;
+
+ /* Assume every character is exactly 2 bytes */
+ i += 2;
+ }

- if (enc_is_utf16(encoding)) {
- /*
- * Assume BOM is always present,
- * and every character is 2 bytes long
- */
- ret *= 2;
- ret += 2;
+ BUG_ON(1);
}
-
- return ret;
}

static void decode_txxx(struct id3tag *id3, const char *buf, int len, int encoding)
@@ -762,7 +768,7 @@ static void decode_txxx(struct id3tag *id3, const char *buf, int len, int encodi
else if (!strcasecmp(out, "compilation"))
key = ID3_COMPILATION;

- size = id3_skiplen(out_mem, encoding);
+ size = id3_skiplen(buf, len, encoding);
free(out_mem);

if (key == NUM_ID3_KEYS)
@@ -924,7 +930,7 @@ static void v2_add_frame(struct id3tag *id3, struct v2_frame_header *fh, const c
encoding = *buf++;
len = fh->size - 1;

- if (encoding > 3)
+ if (encoding > ID3_ENCODING_MAX)
return;

idx = frame_tab_index(fh->id);
--
1.7.1
Gregory Petrosyan
2011-04-09 17:57:51 UTC
Permalink
- correctly skip the comment description
- do not use comments with non-empty description, they are not for end-user

Also bump cache version, to force tags re-reading.

Signed-off-by: Gregory Petrosyan <***@gmail.com>
---
cache.c | 2 +-
id3.c | 24 +++++++++++++++++++++---
2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/cache.c b/cache.c
index 5b7dfb2..48f91c8 100644
--- a/cache.c
+++ b/cache.c
@@ -241,7 +241,7 @@ int cache_init(void)
cache_header[4] = flags & 0xff;

/* assumed version */
- cache_header[3] = 0x04;
+ cache_header[3] = 0x05;

cache_filename = xstrjoin(cmus_config_dir, "/cache");
return read_cache();
diff --git a/id3.c b/id3.c
index 5609cd9..0df908f 100644
--- a/id3.c
+++ b/id3.c
@@ -788,15 +788,33 @@ static void decode_txxx(struct id3tag *id3, const char *buf, int len, int encodi

static void decode_comment(struct id3tag *id3, const char *buf, int len, int encoding)
{
+ int slen;
char *out;

- if (len <= 4)
+ if (len <= 3)
return;

/* skip language */
- buf += 4;
- len -= 4;
+ buf += 3;
+ len -= 3;

+ /* "Short content description" part of COMM frame */
+ out = decode_str(buf, len, encoding);
+ if (!out)
+ return;
+
+ /* we are interested only in comments with empty description */
+ if (*out != '\0')
+ return;
+
+ slen = id3_skiplen(buf, len, encoding);
+ if (slen >= len)
+ return;
+
+ buf += slen;
+ len -= slen;
+
+ free(out);
out = decode_str(buf, len, encoding);
if (!out)
return;
--
1.7.1
Loading...