1.
116         byte[] buffer = new byte[length];

I would like to use a fixed length, such as 1024, for memory saving for large data. Just a very very personal preference.

2. 562 ~ 567
You assume that the end character of a line is the same for all lines. That's OK, I just worry about the situation that a program generated input steam, which would include more than one of CR, LF, CR+LF as line end character. Hopefully, it is not a issue.

3. 584 ~ EOF
You assume that the tag occupy only one byte, that's incorrect, the tag would occupy more than one byte when it is bigger than 30. The assume would make the following length parser code incorrect.

You assume that the end of indefinite length is only one zero byte, that's incorrect, it is zero of two bytes.

You does not handle content longer than 10K(L>0x82). I don't think 10K is enough, maybe you need to handle 0x83, even 0x84 before regarding the content is too hug.

Weijun Wang wrote:
Hi Sean and/or Andrew

Can any of you take a review at this bug fix:

  bug:
     http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6813340
  webrev:
     http://cr.openjdk.java.net/~weijun/6813340/webrev.02/

The bug is about too many is.available() usage in
X509Factory.generateXXX() methods. This means a slow stream might not be
consumed at all.

The fix introduces a new method readOneBlock() which reads a block of
data, either PEM or DER, in block mode. The method neither uses
available() nor performs any mark/reset actions.

There might be two drawbacks for this code change:

1. In order to avoid mark/reset, it uses a heuristic method to detect
the line ending of a PEM file: If the -----BEGIN----- line ends with
'\r' (or \n), it assumes the -----END---- line also ends with it. I
don't know if there are files which is hybrid. I might make another
change if you want more safety:

564  -               if (next == -1 || next == end) {
564  +               if (next == -1 || next == end || next == '\n') {

See above, I would like the safer one.

Andrew
2. Since no buffering is made, the performance might hurt. However, I
simply browse the usage in JDK and find many callers actually use a
ByteArrayInputStream, so this is not a serious problem.

Thanks
Max

Reply via email to