RFR JDK-8200530: '\r' is not supported as "newline" in java.util.jar.Manifest.

[Jim Laskey]

Attributes.java:380 nit

- assign c at decl
- only test len if decremented 

>>>>>>
            byte c;
            if ((c = lbuf[--len]) != '\n' && c != '\r') {
                throw new IOException("line too long");
            }
            if (len > 0 && lbuf[len-1] == '\r') {
                --len;
            }
            if (len == 0) {
                break;
            }
======
            byte c = lbuf[--len];
            if (c != '\n' && c != '\r') {
                throw new IOException("line too long");
            }
            if (len > 0 && lbuf[len-1] == '\r' && --len == 0) {
                break;
            }
<<<<<<


Manifest.java:208 nit

- same as above

>>>>>>
            byte c;
            if ((c = lbuf[--len]) != '\n' && c != '\r') {
                throw new IOException("manifest line too long");
            }
            if (len > 0 && lbuf[len-1] == '\r') {
                --len;
            }
            if (len == 0 && skipEmptyLines) {
                continue;
            }
======
            byte c = lbuf[--len];
            if (c != '\n' && c != '\r') {
                throw new IOException("manifest line too long");
            }
            if (len > 0 && lbuf[len-1] == '\r' && --len == 0 && skipEmptyLines) {
                continue;
            }
<<<<<<

Manifest.java:396 nit

- Shouldn’t c already be loaded with tbuf[tpos-1] (or tbuf[tpos-2]) if “\r\n”)

>>>>>>
                if ((c = tbuf[tpos-1]) == '\n' || c == '\r') {
                    break;
                }
======
                if (c == '\n' || c == '\r') {
                    break;
                }
<<<<<<


+1

Cheers,

— Jim


> On Jun 5, 2018, at 2:38 AM, Xueming Shen <xueming.shen at oracle.com> wrote:
> 
> Hi,
> 
> Please help review the changeset for JDK-8200530.
> 
> "newline" is specified as |CR LF | LF | CR|(/not followed by/|LF|) in Jar spec [1] from
> the very beginning but our implementation actually never supports "\r"/CR (not
> followed by LF) case. The proposed change here is to add CR as an individual
> supported "newline"/line separator.
> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8200530
> webrev: http://cr.openjdk.java.net/~sherman/8200530/webrev
> 
> Thanks,
> Sherman
> 
> 
> [1] https://docs.oracle.com/javase/10/docs/specs/jar/jar.html