Hi,

One of the teams we work with was recently complaining that whenever
they pass an already-encoded URL to <h:graphicImage> that they get a
double encoding, which never happened before in our previous release
(which corresponded to the 1.2 timeframe).  It appears that this is
the result of a bug in encodeURIAttribute(), introduced after 1.2 and
before 2.0:

                    else if (c == '%')
                    {
                        if (i + 2 < string.length())
                        {
                            char c1 = string.charAt(i+1);
                            char c2 = string.charAt(i+2);
                            if ((( c1 >= '0' && c1 <='9') || (c1 >='A' && c1 
<='Z')) &&
                                (( c2 >= '0' && c2 <='9') || (c2 >='A' && c2 
<='Z')))
                            {
                                // do not percent encode, because it could be
already encoded
                                // and we don't want encode it twice
                            }
                            else
                            {
                                app = percentEncode(c, UTF8);
                            }
                        }
                        else
                        {
                            app = percentEncode(c, UTF8);
                        }
                    }

(Forgive me if the formatting doesn't come out right...)

So whenever we find a %, we try to look ahead and see if the next two
characters constitute two hex characters.  If so, we leave the % alone
since this is clearly a portion of the string that has already been
encoded.

There are two problems with that code however:

1. Checking for '0' <= c <= '9' && 'A' <= c <= 'Z' means that "%XX"
would remain unchanged, when really it needs to be "%2AXX".  We should
be checking for 'A' <= c <= 'F' since we're talking about hex
characters.
2. The problem our colleagues saw: we aren't handling lowercase characters!

So I just want to make sure everyone is in agreement with my
assessment that we need to change the character range from A..Z to
A..F and also check for a..f.  If so, I'll make the change.

Thanks,


Curtiss Howard

Reply via email to