Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 358121

Summary: Implement new UTF8 Algorithm to UTF8Appendable.java
Product: [RT] Jetty Reporter: Thomas Becker <tbecker>
Component: otherAssignee: Simone Bordet <simone.bordet>
Status: RESOLVED FIXED QA Contact:
Severity: minor    
Priority: P3 CC: gregw, simone.bordet
Version: unspecified   
Target Milestone: 7.5.x   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Patch files
none
proposed patch none

Description Thomas Becker CLA 2011-09-19 10:13:05 EDT 
Build Identifier: 

Replace the existing UTF8Appendable.java with the algorithm from Bjoern Hoehrmann described here:

http://bjoern.hoehrmann.de/utf-8/decoder/dfa/

Measure the performance differences and add Unit tests for invalid UTF8 chars.

Reproducible: Always
Comment 1 Thomas Becker CLA 2011-09-19 10:14:24 EDT 
Created attachment 203595 [details]
Patch files

Tar archive containing git patch files.
Comment 2 Greg Wilkins CLA 2011-09-20 07:35:21 EDT 
Thomas,

the patch file is a little strange... it appears to have the complete history of the development of the change.  It would be best to get just a diff from trunk to the new code rather than the intermediary steps.

Also, I think you need to do more with regards to the copy right.  If you read the dfa page you will see a license statement that says:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

So can you make sure you have the full license in the file.
Comment 3 Thomas Becker CLA 2011-09-20 08:19:20 EDT 
The patch includes a single file per commit. That's ok and intended, so you keep the whole change history. If I do an individual commit for a code format for example, it's necessary to keep it separated from other commits. 

However the patch is broken as I didn't have a clean copy of origin/master, but my local master copy of the git repo including some changes from another patch.

I've redone the whole patch based on trunk and will attach the new patch file in a minute. I also added the full copyright notice now.
Comment 4 Thomas Becker CLA 2011-09-20 08:25:09 EDT 
Created attachment 203673 [details]
proposed patch

New patch file based on origin/master
Comment 5 Simone Bordet CLA 2011-09-21 06:27:05 EDT 
Patch reviewed and applied.