Gargoyle Forum

You're absolutely right -- your unescape strategy is better, because it should be able to handle all escaped characters, not just the special ones that need to be escaped.

However, there is a bug in your code. There is an edge case where it fails. Consider the following escaped string: "abc%2540def%2540ghi"

%25 is the escape sequence for the "%" character and %40 is the escape sequence for the "@" character.

This should be unescaped to: "abc%40def%40ghi" Your code will transform this to "abc@def@ghi".

I've updated your proposed function to fix this error by making sure "%" gets unescaped last, and replaced my old implementation.

Thanks for your contribution! In case you want to become a regular contributor this counts as your first patch. My policy is to grant anyone who contributes 3 patches that are accepted commit access to the SVN.

very thanks for your proposal: I'll keep in mind

about the code: are u sure the string above is bad committed ?
% is analyzed once because if encountered s is incremented of all the code, pointing next to %xx
ex blablabla%2540 start checking escape when *s = '%' but in the next loop stroke it points *s = '4' (s+=2 when *s == '%' and then s++ at the end of the code block on for cycle)

look at the code involved:

for(s=word;*s != '\0';s++) { //analyze each single word character
if(*s == '%') { //hex-encoded char founded
char old[4];
char new[2];
char* new_modword;
strncpy(old, s, 3); //copy old hex code from word
old[3]='\0'; //make sure to terminate string after %xx (strncpy doesn't add '\0')
s+=2; //point string over hex code (avoid 2 next useless checkouts)
sprintf(new, "%c", (old[1]>'9'?(old[1]&0xDF)-'A'+10:old[1]-'0')*16+(old[2]>'9'?(old[2]&0xDF)-'A'+10:old[2]-'0')); //format conversion
new_modword = replace_str(modword, old, new);
free(modword);
modword = new_modword;
}
}

let's see what happen with: "abc%2540def%2540ghi"
s="abc%2540def%2540ghi"
... (for loop leaves word unaltered until % is reached)
*s='%' --> old="%25\0"
s=s+2='%'+2='5'
new="%" [(old[1]>'9'?(old[1]&0xDF)-'A'+10:old[1]-'0')*16+(old[2]>'9'?(old[2]&0xDF)-'A'+10:old[2]-'0')]
new_modword=replace_str("abc%2540def%2540ghi", "%25\0", "%")
modword="abc%40def%40ghi"
s=s+1='5'+1='4'
*s!='\0'
and so on until the for loop end leaving string modword unaltered
% is parserized once and only once because s pointer andvances to the end of the escaping code

maybe you consider another point of code where applying it
I applied it in my patch just before preparing plain_auth string so it's applied only once per url request

Ah, I am right, I just didn't suggest an appropriate test case. You're right that that case will work, but this one won't: "abc%2540def%40ghi"

Since the %25 comes first, it gets substituted. Then we see %40, and all instances of %40 (which now includes what used to be %2540) get substituted with '@' since replace_str() replaces all instances. So instead of "abc%40def@ghi" we get "abc@def@ghi"