[rfc][icedtea-web] Console Output Encoding Fix

[Jiri Vanek]

On 07/15/2014 04:22 PM, Jiri Vanek wrote:
> On 07/15/2014 04:10 PM, Jie Kang wrote:
>> Hello,
>>
>> This patch resolves the bug here http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1858
>>
>> Characters such as 'ó' were not appearing in the Java Console due to the implementation of
>> TeeOutputStream appending bytes to a StringBuffer in a byte-by-byte fashion ignoring the fact that
>> the encodings involve multi-byte characters.
>>
>> Also, as far as I can tell the StringBuffer is not used by multiple threads and has been replaced
>> by StringBuilder (see http://docs.oracle.com/javase/7/docs/api/java/lang/StringBuilder.html)
>>
>>
>> Regards,
>>
>
>
> Hi!
>
> I like this patch!
>
> Few nits:
>
>
> This willneed backport to 1.5. Although I strongly agree with StringBuffer->StringBuilder  change, I
> would be more happy if 1.5 donot get this change, but stays with Buffer (???).
>
>
> +    private void appendString(String s) {
> +        string.append(s);
> +        flushLog();
> +    }
>
> Why are you flushing there? I thing the flushing is done right. Have you encountered some  case when
> some logs were missing?
>
> @@ -90,9 +90,11 @@
>           } else if (len == 0) {
>               return;
>           }
> -        for (int i = 0; i < len; i++) {
> -            appendChar(b[off + i]);
> +        byte[] offsetString = new byte[len];
> +        for (int i=0; i<len; i++) {
> +            offsetString[i] = b[off+i];
>           }
> +        appendString(new String(offsetString));
>           super.write(b, off, len);
>       }
>
>
> Looking onto this, I'mnot sure how correct it is. I'm a bit afraid it may not handle various
> encoding correctly (/me must try and check) ).  Also I need to check  the performancy here.
>
>
> And most uncomfortable nit last:
> You can see that teeOutputStream is lacking unittests. This is great opportunity to add some. And
> also cover the 1858.
>
>
> ty!
>
>   J.
>

Ok. I looked a bit more inside, and I might be wrong, but this do not seam to me complete fix.

First issue - you fixed only

write(byte[] b, int off, int len) {

but the

void write(int b) {

maybe affected in same way.

Also the write(byte[] b, int off, int len)  bay end in middle of character.

I think the only correct solution here is to have bytebuffer, to append individidual bytes, and once 
the last byte is \n (yes it have its just number) then pretend it is line-end, and convert whole 
bytebuffer to String. (and yes, probably stay with platform default encoding)

I think it needs physical redesign of this class.


I'm not sure if my suggestion is overkilling, or if I'm not judging your approach wrongly. I would 
be happy for more voices and sorry if I do.

J.

ps: this doubles the need of unittests.