[OWASP-ESAPI] Encoding implementation issue...

'Ed Schaller' schallee at darkmist.net
Wed Dec 9 16:01:07 EST 2009


> ... and these characters are used rarely, so I feel that encoding them,
> anyhow, will have minimal performance impact. I say we go for it. I'd
> rather someone complain about performance (Like Knuth said, get it right
> first, optimize later), than under-aggressive encoding.

The fix is committed. It took me longer than it should have because I got
screwed up thinking that that method was designed to always return zero
padded two characters. The replacement just creates a new hex string
instead of hitting the pre-build array so the performance should only
change for > 0xff. The java doc for the method didn't mention different
behavior for > 0xff so with the fix it does exactly what the java doc
said it did. The toHex(char) and toOct(char) also had the same issue so
I fixed them as well.

PercentCodec had more issues because chars outside of the ASCII range
need to be converted to UTF-8 and then have each byte encoded.

I'd be curious to know if folks have ever had issues with performance
in the encoders. There is a fair bit that could be done to make them
faster but I'm not sure it's worth it unless there are really issues
there. It would, for example, be fairly easy to cache results in a
memory sensitive manner so that frequently used ones would just be a
lookup. Similarly, modifications to the interface so that StringBuilders
or even just Appendables could be passed in and be appended to instead
of creating new strings each time. Optimization is a really good way to
screw things up though so I think having more comprehensive unit tests
would be good first.

I have not looked very closely at 1.4 on this issue as it's codecs are
implemented very differently and in some cases use different methods of
encoding. I did port CodecTest to it and not surprising a lot of the
tests fail. The CSSCodec in 1.4 also appeared to have code originally
from the JavaScriptCodec that I believe is incorrect but I would need
to pull out the CSS spec to check.

>>>------>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
Url : https://lists.owasp.org/pipermail/owasp-esapi/attachments/20091209/7b6be05f/attachment.bin 


More information about the OWASP-ESAPI mailing list