Re: [PATCH] - improved decoding of URL special characters

Hi,

Why is text in urldecodestr a char ** and not char *?  It never sets *text to
anything, so there's no point in making it char **, unless I'm not seeing
something.

My humble opinion is that a better urldecodestr()-type function would look
like this.  Notably, let the libc do the hex conversion.  This also does
less copies if the string consists of more than one %xx sequence.

But.  This is just my opinion.

#include <stdio.h>	/* for sscanf */
#include <ctype.h>	/* for isxdigit */

/*
 * Modify the buffer buf to convert %xx into the appropriate character.
 */
int urldecodestr( char *buf )
{
	const char *r = buf;
	while( *r )
		switch( *r )
		{
		  case '%':
			/* Is this really a 2-digit hex string? */
			if( isxdigit(r[1]) && isxdigit(r[2]) )
			{
				char hex[3];
				int i;
				hex[0] = *++r;
				hex[1] = *++r;
				hex[2] = 0;
				++r;
				sscanf( hex, "%x", &i );
				*buf++ = i; 
				break;
			}
			/* No, follow through. */
		  default:
			*buf++ = *r++;
		}
	*buf = 0;
}

On Wed, Oct 19, 2005 at 01:19:54PM -0500, Dustin Kirkland wrote:
> Hi-
> 
> I've attached a small patch that improves the decoding of URL special
> characters.  XMMS has long supported converting "%20" into a blank
> space.  However, other multibyte characters (apostrophes, parentheses,
> etc) are not converted in the same manner.
> 
> I first reported this problem 2002-10-24 in a bugzilla, with a
> suggested fix, but not a completed patch:
> http://bugs.xmms.org/show_bug.cgi?id=1068
> 
> I later reworked the patch as below, which has worked very well for me
> for several years now.  I would like to see this accepted in the
> upstream XMMS.
> 
> There are two helper functions that are added by this patch, axtoi()
> and urldecodestr() in urldecode.c.
> 
> The first, axtoi() takes a string pointer as input and returns an
> integer.  It will calculate the decimal integer value of a 2-character
> hexadecimal string, such as "20" -> 32, or "2A" -> 42.
> 
> The second, urldecodestr(), takes a pointer to a string pointer, and
> searches through the string for the % character.  When this is found,
> the location is passed to the axtoi() function, which will attempt to
> convert the following two characters to an integer.  If it is able to
> do so, the "%XX" three characters in the string are replaced with the
> appropriate character that was url-encoded.  The string passed in is
> modified (rather than mallocing a new pointer).
> 
> Finally, the few places in the code where the %20-to-whitespace
> conversion happened were replaced with a call to urldecodestr().
> 
> This provides support for several characters that are frequently found
> in mp3 filenames besides the blank space, such as: the apostrophe,
> comma, tilda, parentheses, brackets, braces, etc.
> 
> For example...
> 
> http://localhost/Bob%20Dylan/Biograph%20%28Disc%203%20of%203%29/15%20-%20Knockin%27%20On%20Heaven%27s%20Door.mp3]
> 
> now translates to the following in the playlist window (with parens
> and tick marks correctly placed)...
> 
> Bob Dylan/Biograph (Disc 3 of 3)/15 - Knockin' On Heaven's Door.mp3
> 
> Comments are welcome.  I would very much like to see this included in
> the next release.
> 
> :-Dustin
> 
> 
> diff -uprN xmms-1.2.10/xmms/playlist.c xmms-1.2.10-hex_to_char/xmms/playlist.c
> --- xmms-1.2.10/xmms/playlist.c	2004-02-23 14:31:43.000000000 -0600
> +++ xmms-1.2.10-hex_to_char/xmms/playlist.c	2005-10-19 04:47:52.000000000 -0500
> @@ -983,16 +983,7 @@ char *playlist_get_info_text(void)
>  		while ((tmp = strchr(text, '_')) != NULL)
>  			*tmp = ' ';
>  	if (cfg.convert_twenty)
> -		while ((tmp = strstr(text, "%20")) != NULL)
> -		{
> -			char *tmp2;
> -			tmp2 = tmp + 3;
> -			*(tmp++) = ' ';
> -			while (*tmp2)
> -				*(tmp++) = *(tmp2++);
> -			*tmp = '\0';
> -		}
> -	
> +		urldecodestr(&text);
>  	return text;
>  }
> 
> diff -uprN xmms-1.2.10/xmms/playlist_list.c
> xmms-1.2.10-hex_to_char/xmms/playlist_list.c
> --- xmms-1.2.10/xmms/playlist_list.c	2003-06-11 13:44:17.000000000 -0500
> +++ xmms-1.2.10-hex_to_char/xmms/playlist_list.c	2005-10-19
> 04:58:47.000000000 -0500
> @@ -314,18 +314,7 @@ void playlist_list_draw_string_wc(PlayLi
>  	}
> 
>  	if (cfg.convert_twenty && len > 2)
> -	{
> -		GdkWChar *wtmp;
> -		while ((wtmp = find_in_wstr(wtext, "%20")) != NULL)
> -		{
> -			GdkWChar *wtmp2 = wtmp + 3;
> -			*(wtmp++) = L' ';
> -			while (*wtmp2)
> -				*(wtmp++) = *(wtmp2++);
> -			*wtmp = L'\0';
> -			len -= 2;
> -		}
> -	}
> +		urldecodestr(&wtext);
>  		
>  	newlen = len + 2;
> 
> @@ -366,14 +355,8 @@ void playlist_list_draw_string(PlayList_
>  		while ((tmp = strchr(text, '_')) != NULL)
>  			*tmp = ' ';
>  	if (cfg.convert_twenty)
> -		while ((tmp = strstr(text, "%20")) != NULL)
> -		{
> -			char *tmp2 = tmp + 3;
> -			*(tmp++) = ' ';
> -			while (*tmp2)
> -				*(tmp++) = *(tmp2++);
> -			*tmp = '\0';
> -		}
> +		urldecodestr(&text);
>  	len = strlen(text);
>  	while (gdk_text_width(font, text, len) > width && len > 4)
>  	{
> diff -uprN xmms-1.2.10/xmms/urldecode.c xmms-1.2.10-hex_to_char/xmms/urldecode.c
> --- xmms-1.2.10/xmms/urldecode.c	2003-05-19 16:22:08.000000000 -0500
> +++ xmms-1.2.10-hex_to_char/xmms/urldecode.c	2005-10-19 04:46:07.000000000 -0500
> @@ -61,3 +61,47 @@ char *xmms_urldecode_path(char *encoded_
>  	
>  	return encoded_path;
>  }
> +
> +
> +/* take a 2-character hex string and return the integer value */
> +int axtoi(char *hexstr) {
> +	int i = 0;
> +	int hexint[2];
> +	hexint[0] = hexint[1] = 0;
> +	while (i < 2) {
> +		if (hexstr[i]=='\0')
> +			break;
> +		else if (hexstr[i] >= '0' && hexstr[i] <= '9' )
> +			hexint[i] = hexstr[i] - '0';
> +		else if (hexstr[i] >='A' && hexstr[i] <= 'F')
> +			hexint[i] = 10 + (hexstr[i] - 'A');
> +		else if (hexstr[i] >='a' && hexstr[i] <= 'f')
> +			hexint[i] = 10 + (hexstr[i] - 'a');
> +		else break;
> +		i++;
> +	}
> +	return hexint[0]*16 + hexint[1];
> +}
> +
> +
> +/* for each "%" found in string, attempt to decode following two bytes and
> +   replace with URL special character */
> +int urldecodestr(char **text) {
> +	char *str, *tmp, *hexstr, *cur;
> +	int i;
> +	str = cur = *text;
> +	while ((hexstr = strchr(str, '%')) != NULL) {
> +		cur = hexstr + 1;
> +		i = axtoi(cur);
> +		if (i > 0) {
> +			tmp = hexstr + 3;
> +			*(hexstr++) = (char)i;
> +			while (*tmp)
> +				*(hexstr++) = *(tmp++);
> +			*hexstr = '\0';
> +		}
> +		str = cur;
> +	}
> +	return 0;
> +}
> +
> diff -uprN xmms-1.2.10/xmms/urldecode.h xmms-1.2.10-hex_to_char/xmms/urldecode.h
> --- xmms-1.2.10/xmms/urldecode.h	2000-05-31 21:56:20.000000000 -0500
> +++ xmms-1.2.10-hex_to_char/xmms/urldecode.h	2005-10-19 04:46:41.000000000 -0500
> @@ -17,3 +17,5 @@
>   */
> 
>  char *xmms_urldecode_path(char *);
> +int axtoi(char *hexstr);
> +int urldecodestr(char **text);
> 
> _______________________________________________
> xmms-devel mailing list
> [email protected]
> http://lists.xmms.org/mailman/listinfo/xmms-devel
>