Re: Feature suggestion: Add static equals methods to Float and Double

2019-01-09 Thread Martin Desruisseaux
Le 08/01/2019 à 19:55, Hans Boehm a écrit :
> The IEEE standard does say that for quiet NaNs, the value (or one of them)
> "should" be preserved by most operations on the quiet NaN. I have not heard
> of implementations violating this for anything other than the "quiet" bit.
> Thus I don't immediately see why it would be problematic to encode an
> explicitly programmer-introduced error cause in the remaining bits (as
> opposed to relying on hardware-generated patterns). I have not seen
> non-testing code that does so, but I would be mildly surprised if it
> doesn't exist somewhere.

I confirm that such code exist and are used in production for 15 years
at least in the GeoTools and Apache Spatial Information System (SIS)
projects. Earth Observation data are often images mixing measurement
values with different kind of missing values. So above-cited projects
perform computation on float primitive types where, for example:

  * Real values are Sea Surface Temperatures values in °C.
  * One NaN bit pattern stands for values missing because the sea was
hidden by a cloud.
  * Another NaN bit pattern stands for values missing because the pixel
is over a land.
  * Another NaN bit pattern stands for values missing because the remote
sensor did not fly over that area.
  * etc.

Actually those data are typically encoded as integers in some file
format where each missing value is associated to a different color
(clouds in white, lands in brown, etc.). But they are converted to float
values for performing calculations (e.g. applying the "gradient
magnitude" operator provided by Java Advanced Imaging library), then
converted back to integers for displaying purpose with
java.awt.image.RenderedImage. It allows the use of mathematical formulas
without special checks for missing values, and still preserve the lands,
clouds, etc. masks in the resulting image. I have never seen yet a lost
of information encoded in quiet NaN values (e.g. I have not seen a "NaN
for land" mutated to a "NaN for cloud"), except if an arithmetic
operation is applied on two different NaN bit patterns.

    Martin




Re: JDK-8210280 - Unnecessary reallocation when invoking HashMap.putAll()

2019-01-09 Thread Michal Vala

ping

On 1/3/19 9:31 PM, Michal Vala wrote:

Hi Martin,

can we please finish this review?

On 12/19/18 6:32 PM, Michal Vala wrote:



On 12/19/18 4:15 PM, Martin Buchholz wrote:

On Wed, Dec 19, 2018 at 6:59 AM Roger Riggs  wrote:


Hi Martin,

It is also useful and conventional to print the seed of the random
so that if necessary it can be reproduced.



For many years, we've been using ThreadLocalRandom for testing, and that
does not allow setting a seed.

I remain unconvinced that saving a seed has value in the real world.  When
a randomized test fails, running it with sufficient iterations has always
worked for me.



What's the reason behind using ThreadLocalRandom?

In my opinion, reproducing the issue is key. One failure of randomized test 
run might be caused by one issue, second run due to another issue. How we 
reproduce then and how we know even how many issues we have? When we're 
running random tests until it pass, it might even hide the issue.


They sure have good value on reveal the issue, but then we have to know how to 
reproduce and what we're searching for.


If this case would not require ThreadLocalRandom and Random is enough, I'd 
like to use that because of benefits I've mentioned.






--
Michal Vala
OpenJDK QE
Red Hat Czech


RE: RFR (XS) 8216413 : Long.parseLong() is specified to throw unless string contains parsable {@code int}; should be {@code long}

2019-01-09 Thread Langer, Christoph
Hi Ivan,

looks good 😊

Best regards
Christoph

> -Original Message-
> From: core-libs-dev  On Behalf
> Of Ivan Gerasimov
> Sent: Mittwoch, 9. Januar 2019 08:19
> To: core-libs-dev@openjdk.java.net
> Subject: RFR (XS) 8216413 : Long.parseLong() is specified to throw unless
> string contains parsable {@code int}; should be {@code long}
> 
> Hello!
> 
> The javadoc for Long.parseLong() mistakenly mentions {@code int} while
> it should have been {@code long}.
> 
> Would you please help review a trivial fix?
> 
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8216413
> WEBREV: http://cr.openjdk.java.net/~igerasim/8216413/00/webrev/
> 
> Thanks in advance!
> 
> --
> With kind regards,
> Ivan Gerasimov



Re: RFR (XS) 8216413 : Long.parseLong() is specified to throw unless string contains parsable {@code int}; should be {@code long}

2019-01-09 Thread Ivan Gerasimov

Thank you Christoph!

Pushed.


On 1/9/19 5:47 AM, Langer, Christoph wrote:

Hi Ivan,

looks good 😊

Best regards
Christoph


-Original Message-
From: core-libs-dev  On Behalf
Of Ivan Gerasimov
Sent: Mittwoch, 9. Januar 2019 08:19
To: core-libs-dev@openjdk.java.net
Subject: RFR (XS) 8216413 : Long.parseLong() is specified to throw unless
string contains parsable {@code int}; should be {@code long}

Hello!

The javadoc for Long.parseLong() mistakenly mentions {@code int} while
it should have been {@code long}.

Would you please help review a trivial fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8216413
WEBREV: http://cr.openjdk.java.net/~igerasim/8216413/00/webrev/

Thanks in advance!

--
With kind regards,
Ivan Gerasimov


--
With kind regards,
Ivan Gerasimov



RFR - JDK-8215489 Remove String::align

2019-01-09 Thread Jim Laskey
Please review the removal of String::align from JDK12.

Thank you.

— Jim


webrev: http://cr.openjdk.java.net/~jlaskey/8215489/webrev/index.html 



JBS: https://bugs.openjdk.java.net/browse/JDK-8215489 

CSR: https://bugs.openjdk.java.net/browse/JDK-8215490 




Re: RFR - JDK-8215489 Remove String::align

2019-01-09 Thread Sundararajan Athijegannathan

Looks good.

-Sundar

On 09/01/19, 8:56 PM, Jim Laskey wrote:

Please review the removal of String::align from JDK12.

Thank you.

— Jim


webrev: 
http://cr.openjdk.java.net/~jlaskey/8215489/webrev/index.html


JBS: 
https://bugs.openjdk.java.net/browse/JDK-8215489
CSR: 
https://bugs.openjdk.java.net/browse/JDK-8215490



Re: JDK-8215626 : Correct [^..&&..] intersection negation behaviour JDK8 vs JDK11 ??

2019-01-09 Thread Andrew Leonard
Thanks Sherman,
Yes I agree.
Cheers
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 




From:   Xueming Shen 
To: core-libs-dev@openjdk.java.net
Date:   08/01/2019 16:50
Subject:Re: JDK-8215626 : Correct [^..&&..] intersection negation 
behaviour JDK8 vs JDK11 ??
Sent by:"core-libs-dev" 



Hi Andrew,


See [1]/[2] for the background of the fix. I would say jdk11 behavior is 
correct

and expected :-) anyway, it's a  behavior change, so probably will not 
be easily

to go back into jdk8.

Regards,

Sherman


[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-June/006957.html


[2] 
http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/lambdafunction



On 1/7/19 5:50 AM, Andrew Leonard wrote:
> Anyone got any views on which "regex" beheviour is correct JDK8 or JDK11 
?
> thanks
> Andrew
>
> Andrew Leonard
> Java Runtimes Development
> IBM Hursley
> IBM United Kingdom Ltd
> Phone internal: 245913, external: 01962 815913
> internet email: andrew_m_leon...@uk.ibm.com
>
>
>
>
> From:   Andrew Leonard/UK/IBM
> To: "OpenJDK Core Libs Developers" 
> Date:   03/01/2019 11:20
> Subject:JDK-8215626 : Correct [^..&&..] intersection negation
> behaviour JDK8 vs JDK11 ??
>
>
> Hi,
> I'm currently investigating bug JDK-8215626 and have discovered the
> problem is in the Pattern interpretation of the [^..&&..] negation when
> applied to "intersected" expressions. So I have simplified the bug 
example
> to a more extreme and obvious example:
>  Input string: "1234 ABCDEFG !$%^& abcdefg"
>  pattern RegEx: "[^[A-B]&&[^ef]]"
>  Operation: pattern.matcher(input).replaceAll("");
>
> JDK8 output:
>1234 CDEFG !$%^& abcdefg
> JDK11 output:
>AB
>
> So from the "spec" :
> A character class is a set of characters enclosed within square 
brackets.
> It specifies the characters that will successfully match a single
> character from a given input string
> Intersection:
> To create a single character class matching only the characters common 
to
> all of its nested classes, use &&, as in [0-9&&[345]].
> Negation:
> To match all characters except those listed, insert the "^" 
metacharacter
> at the beginning of the character class.
>
> The way I read the "spec" is the "^" negation negates the whole 
character
> class within the outer square brackets, thus in this example:
>  "[^[A-B]&&[^ef]]"  is equivalent to the negation of 
"[[A-B]&&[^ef]]"
>  ie.the negation of the intersect of chars A,B and everything other
> than e,f
>  which is thus the negation of A,B
>  hence the operation above will remove any character in the input
> string other than A,B
> Hence, JDK11 in my opinion meets the "spec". It looks as though JDK8 is
> applying the ^ negation to just [A-B] and then intersecting it with 
[^ef],
> which to me is the wrong interpretation of the "spec".
>
> Your thoughts please?
>
> If JDK11 is correct, and JDK8 wrong, then the next question is do we fix
> JDK8? as there's obviously potential "behavioural" impacts to existing
> applications?
>
> Thanks
> Andrew
>
> Andrew Leonard
> Java Runtimes Development
> IBM Hursley
> IBM United Kingdom Ltd
> Phone internal: 245913, external: 01962 815913
> internet email: andrew_m_leon...@uk.ibm.com
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU
>
>
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU





Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


Re: [RFR] 8214440: ldap over a TLS connection negotiate failed with "javax.net.ssl.SSLPeerUnverifiedException: hostname of the server '' does not match the hostname in the server's certificate"

2019-01-09 Thread Rob McKenna
The parameter can be null, but if you look at the spec for getDomainName
it details the behaviour when the result is created with a null value.

I would rather avoid changing the spec at this point so I plan to stick
with this approach. (I'll switch the "".equals for isEmpty - thanks for
that)

-Rob

On 09/01/19 12:33, vyom tewari wrote:
> Hi Rob,
> 
> Thanks for fixing this issue, please use hostname.isEmpty(), instead of
> "".equal(hostname). I have a question to you, why we are converting null to
> empty string("") in LdapDnsProvider ?.
> 
> If you see the java doc it tells that domainname can be null
> 
> /**
>  * Construct an LdapDnsProviderResult consisting of a resolved domain
> name
>  * and the ldap server endpoints that serve the domain.
>  *
>  * @param domainName    the resolved domain name; can be null.
> 
> My personal suggestion is not to replace null to empty string("") in
> "LdapDnsProviderResult" either throw some exception or just pass whatever
> you got in LdapDnsProviderResult constructor.
> 
> Thanks,
> 
> Vyom
> 
> On 08/01/19 10:33 PM, Rob McKenna wrote:
> > Hi folks,
> > 
> > I'd like to fix this test failure caused by 8160768.
> > 
> > The problem is that the LdapDnsProviderResult sets the hostname to the
> > empty String and gets passed to StartTlsResponseImpl.verify.
> > Unfortunately StartTlsResponseImpl.verify only expects null values.
> > Since null and the empty String are functionally equivalent I've added a
> > check to StartTlsResponseImpl.verify to take the empty String into
> > account.
> > 
> > http://cr.openjdk.java.net/~robm/8214440/webrev.01/
> > 
> > This was caught by an existing test which I managed to miss in my
> > testing incantation.
> > 
> >  -Rob
> > 


Re: [RFR] 8214440: ldap over a TLS connection negotiate failed with "javax.net.ssl.SSLPeerUnverifiedException: hostname of the server '' does not match the hostname in the server's certificate"

2019-01-09 Thread Xue-Lei Fan
The behavior looks similar to the underlying TLS implementation.  Looks 
good to me.


Thanks,
Xuelei

On 1/8/2019 9:03 AM, Rob McKenna wrote:

Hi folks,

I'd like to fix this test failure caused by 8160768.

The problem is that the LdapDnsProviderResult sets the hostname to the
empty String and gets passed to StartTlsResponseImpl.verify.
Unfortunately StartTlsResponseImpl.verify only expects null values.
Since null and the empty String are functionally equivalent I've added a
check to StartTlsResponseImpl.verify to take the empty String into
account.

http://cr.openjdk.java.net/~robm/8214440/webrev.01/

This was caught by an existing test which I managed to miss in my
testing incantation.

 -Rob



Re: [RFR] 8216362: Incorrect jar file error message when there is an invalid manifest

2019-01-09 Thread Sean Mullan

On 1/8/19 7:17 PM, Philipp Kunz wrote:
Manifest.read throws an exception with the jar file name passed to the 
constructor the manifest was constructed with and not passed to the call 
to the read that throws the exception because the jarFilename variable 
is not reset after successful construction with read and will be used by 
subsequent calls to read if read is called (again) after the manifest 
has been constructed. The call to the constructor could be in a 
different context than the call to read and the jar file name could 
therefore be exposed in an unexpected context. After I first thought it 
was just annoying to get an unrelated jar file name in an exception 
message I see now a security concern as well.


That's a good point (and good catch!). I think we need to adjust the 
code so that if read is called and it throws an Exception it only 
contains the jar file name if called by the constructors in which the 
jar file name is passed as a parameter. Perhaps break up the read method 
into a private and public one with the private one containing an 
additional boolean parameter that is set to true if called by the 
constructor, otherwise it is false. If the boolean parameter is true, 
the jar file name is put in the exception, otherwise it is not.


I also think we should fix this in 12, so I raised the priority to 3.

--Sean


At first glance and in terms of expectable code changes to the 
questionable constructor of Manifest the above mentioned seems to be 
overlapping with issue JDK-8216362 but then JDK-8216362 is about the jar 
file name missing in an error message when it should be present and not 
the other way round. Attached is a patch for JDK-8216362 as it is 
described at the moment.


Philipp


On Tue, 2019-01-08 at 13:07 -0500, Sean Mullan wrote:

In this case, the caller is passing in the filename through the public
JarFile API so as long as it is not modified it should be ok. The
concerns I raised previously are situations where the caller did not
pass in the file or the JDK converts a relative path to an absolute
path, which could reveal sensitive details about the filesystem.

--Sean


[PATCH] JDK-8216140: Correct UnicodeDecoder U+FFFE handling

2019-01-09 Thread Giovanni Gatti Pinheiro
Hello,

I’ve crossed this bug on the past and I would like to fix it. You will find
the patch with the fix attached to this message.

I have few questions around this subject.

1.Where should the test be placed?
It is not clear to me which is the standard approach. I’ve spent about 1h
searching in the code base where it should be placed without success. Then
I’ve tried to « fix » the code and to run nio tests expecting to break
something (however no test failed). The closest I’ve got from a reasonable
place is TestUTF_16 class. In this class, there is a test for « Reversed
BOM in middle of stream Negative test. » `that is commented out, which is
the opposite to what I’m tried to do.

2.What to do with UTF-8/UTF-32?
I’ve tested UTF-8/UTF-32 to see how these two implementations handle U+FFFE
in the middle of a byte stream. They are both compliant with Unicode
specification and it looks like that this bug applies only to UTF-16’s
implementation. It’s awkward that these three encodings do not behave the
same way, so I would like to confirm with you that I don’t have to do
anything special about it.

3.What exactly should I test?
Technically I should’ve tested that all Unicode non-characters are not
reported as malformed input. Do I have to go that far or just testing
U+FFFE is enough?

4.Do I have to sign OCA document to this contribution?
It’s really a small fix and I don’t really care about any credit. But
still, if I must, just let me know and I will do it ASAP.

So, let me know what to do exactly and I will do take care about it.

Thank you in advance.

Best regards,

Giovanni GATTI PINHEIRO
diff --git a/src/java.base/share/classes/sun/nio/cs/UnicodeDecoder.java b/src/java.base/share/classes/sun/nio/cs/UnicodeDecoder.java
index c3509d7..e6bcef7 100644
--- a/src/java.base/share/classes/sun/nio/cs/UnicodeDecoder.java
+++ b/src/java.base/share/classes/sun/nio/cs/UnicodeDecoder.java
@@ -91,11 +91,6 @@ abstract class UnicodeDecoder extends CharsetDecoder {
 
 char c = decode(b1, b2);
 
-if (c == REVERSED_MARK) {
-// A reversed BOM cannot occur within middle of stream
-return CoderResult.malformedForLength(2);
-}
-
 // Surrogates
 if (Character.isSurrogate(c)) {
 if (Character.isHighSurrogate(c)) {
diff --git a/test/jdk/sun/nio/cs/TestUTF_16.java b/test/jdk/sun/nio/cs/TestUTF_16.java
index 25344dd..a3f3e2d 100644
--- a/test/jdk/sun/nio/cs/TestUTF_16.java
+++ b/test/jdk/sun/nio/cs/TestUTF_16.java
@@ -184,7 +184,24 @@ public class TestUTF_16 {
throw new Exception ("Incorrectly parsed BOM in middle of input");
 */
 
-// Fixed included with bug 4403848 fixes buffer sizing
+// Test 7: Decoding does not report unicode non-character (U+FFFE)
+if (StandardCharsets.UTF_16LE.newDecoder()
+.onMalformedInput(CodingErrorAction.REPORT)
+.decode(ByteBuffer.allocate(6)
+.put(new byte[]
+{(byte) 0x61, (byte) 0x00,
+(byte) 0xfe, (byte) 0xff,
+(byte) 0x64, (byte) 0x00})
+.flip(),
+CharBuffer.allocate(3),
+true)
+.isMalformed()) {
+
+throw new Exception("REGTEST TestUTF16 non-character U+FFFE test failed");
+}
+
+
+// Fixed included with bug 4403848 fixes buffer sizing
 // issue due to non provision of additional 2 bytes
 // headroom for initial BOM bytes for UTF-16 encoding.
   System.err.println ("OVERALL PASS OF UTF-16 Test");


Re: RFR: JDK-8215510: j.l.c.ClassDesc is accepting descriptors not allowed by the spec

2019-01-09 Thread Brian Goetz

+1 from me.

On 1/7/2019 1:17 PM, Vicente Romero wrote:
I have updated the webrev after additional feedback from the TCK 
tester please check last version at [1]


Thanks,
Vicente

[1] http://cr.openjdk.java.net/~vromero/8215510/webrev.01


On 1/3/19 12:21 PM, Vicente Romero wrote:
Please review the fix for bug [1] at [2]. Basically the constants API 
introduced as part of JEP-334 [3] were accepting descriptors and 
class names not allowed by the spec. This implementation fixes 
several issues found by TCK tests on JEP-334,


Thanks,
Vicente

[1] https://bugs.openjdk.java.net/browse/JDK-8215510
[2] http://cr.openjdk.java.net/~vromero/8215510/webrev.00/
[3] https://bugs.openjdk.java.net/browse/JDK-8203252






Re: [RFR] 8216362: Incorrect jar file error message when there is an invalid manifest

2019-01-09 Thread Lance Andersen
Here is the webrev for the changes:

 http://cr.openjdk.java.net/~lancea/8216362/webrev.00/index.html 


Best
Lance
> On Jan 9, 2019, at 12:13 PM, Sean Mullan  wrote:
> 
> On 1/8/19 7:17 PM, Philipp Kunz wrote:
>> Manifest.read throws an exception with the jar file name passed to the 
>> constructor the manifest was constructed with and not passed to the call to 
>> the read that throws the exception because the jarFilename variable is not 
>> reset after successful construction with read and will be used by subsequent 
>> calls to read if read is called (again) after the manifest has been 
>> constructed. The call to the constructor could be in a different context 
>> than the call to read and the jar file name could therefore be exposed in an 
>> unexpected context. After I first thought it was just annoying to get an 
>> unrelated jar file name in an exception message I see now a security concern 
>> as well.
> 
> That's a good point (and good catch!). I think we need to adjust the code so 
> that if read is called and it throws an Exception it only contains the jar 
> file name if called by the constructors in which the jar file name is passed 
> as a parameter. Perhaps break up the read method into a private and public 
> one with the private one containing an additional boolean parameter that is 
> set to true if called by the constructor, otherwise it is false. If the 
> boolean parameter is true, the jar file name is put in the exception, 
> otherwise it is not.
> 
> I also think we should fix this in 12, so I raised the priority to 3.
> 
> --Sean
>> At first glance and in terms of expectable code changes to the questionable 
>> constructor of Manifest the above mentioned seems to be overlapping with 
>> issue JDK-8216362 but then JDK-8216362 is about the jar file name missing in 
>> an error message when it should be present and not the other way round. 
>> Attached is a patch for JDK-8216362 as it is described at the moment.
>> Philipp
>> On Tue, 2019-01-08 at 13:07 -0500, Sean Mullan wrote:
>>> In this case, the caller is passing in the filename through the public
>>> JarFile API so as long as it is not modified it should be ok. The
>>> concerns I raised previously are situations where the caller did not
>>> pass in the file or the JDK converts a relative path to an absolute
>>> path, which could reveal sensitive details about the filesystem.
>>> 
>>> --Sean

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 





Re: [RFR] 8216362: Incorrect jar file error message when there is an invalid manifest

2019-01-09 Thread Sean Mullan

Looks good.

--Sean

On 1/9/19 3:42 PM, Lance Andersen wrote:

Here is the webrev for the changes:

http://cr.openjdk.java.net/~lancea/8216362/webrev.00/index.html

Best
Lance
On Jan 9, 2019, at 12:13 PM, Sean Mullan > wrote:


On 1/8/19 7:17 PM, Philipp Kunz wrote:
Manifest.read throws an exception with the jar file name passed to 
the constructor the manifest was constructed with and not passed to 
the call to the read that throws the exception because the 
jarFilename variable is not reset after successful construction with 
read and will be used by subsequent calls to read if read is called 
(again) after the manifest has been constructed. The call to the 
constructor could be in a different context than the call to read and 
the jar file name could therefore be exposed in an unexpected 
context. After I first thought it was just annoying to get an 
unrelated jar file name in an exception message I see now a security 
concern as well.


That's a good point (and good catch!). I think we need to adjust the 
code so that if read is called and it throws an Exception it only 
contains the jar file name if called by the constructors in which the 
jar file name is passed as a parameter. Perhaps break up the read 
method into a private and public one with the private one containing 
an additional boolean parameter that is set to true if called by the 
constructor, otherwise it is false. If the boolean parameter is true, 
the jar file name is put in the exception, otherwise it is not.


I also think we should fix this in 12, so I raised the priority to 3.

--Sean
At first glance and in terms of expectable code changes to the 
questionable constructor of Manifest the above mentioned seems to be 
overlapping with issue JDK-8216362 but then JDK-8216362 is about the 
jar file name missing in an error message when it should be present 
and not the other way round. Attached is a patch for JDK-8216362 as 
it is described at the moment.

Philipp
On Tue, 2019-01-08 at 13:07 -0500, Sean Mullan wrote:

In this case, the caller is passing in the filename through the public
JarFile API so as long as it is not modified it should be ok. The
concerns I raised previously are situations where the caller did not
pass in the file or the JDK converts a relative path to an absolute
path, which could reveal sensitive details about the filesystem.

--Sean




Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 





Re: [RFR] 8216362: Incorrect jar file error message when there is an invalid manifest

2019-01-09 Thread Roger Riggs

+1

On 01/09/2019 03:46 PM, Sean Mullan wrote:

Looks good.

--Sean

On 1/9/19 3:42 PM, Lance Andersen wrote:

Here is the webrev for the changes:

http://cr.openjdk.java.net/~lancea/8216362/webrev.00/index.html

Best
Lance
On Jan 9, 2019, at 12:13 PM, Sean Mullan > wrote:


On 1/8/19 7:17 PM, Philipp Kunz wrote:
Manifest.read throws an exception with the jar file name passed to 
the constructor the manifest was constructed with and not passed to 
the call to the read that throws the exception because the 
jarFilename variable is not reset after successful construction 
with read and will be used by subsequent calls to read if read is 
called (again) after the manifest has been constructed. The call to 
the constructor could be in a different context than the call to 
read and the jar file name could therefore be exposed in an 
unexpected context. After I first thought it was just annoying to 
get an unrelated jar file name in an exception message I see now a 
security concern as well.


That's a good point (and good catch!). I think we need to adjust the 
code so that if read is called and it throws an Exception it only 
contains the jar file name if called by the constructors in which 
the jar file name is passed as a parameter. Perhaps break up the 
read method into a private and public one with the private one 
containing an additional boolean parameter that is set to true if 
called by the constructor, otherwise it is false. If the boolean 
parameter is true, the jar file name is put in the exception, 
otherwise it is not.


I also think we should fix this in 12, so I raised the priority to 3.

--Sean
At first glance and in terms of expectable code changes to the 
questionable constructor of Manifest the above mentioned seems to 
be overlapping with issue JDK-8216362 but then JDK-8216362 is about 
the jar file name missing in an error message when it should be 
present and not the other way round. Attached is a patch for 
JDK-8216362 as it is described at the moment.

Philipp
On Tue, 2019-01-08 at 13:07 -0500, Sean Mullan wrote:
In this case, the caller is passing in the filename through the 
public

JarFile API so as long as it is not modified it should be ok. The
concerns I raised previously are situations where the caller did not
pass in the file or the JDK converts a relative path to an absolute
path, which could reveal sensitive details about the filesystem.

--Sean



 

Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 







Re: RFR: JDK-8215510: j.l.c.ClassDesc is accepting descriptors not allowed by the spec

2019-01-09 Thread John Rose
Nice work.  A couple of small points:

A qualified class name is one that has a package prefix.
So it is surprising that verifyUnqualifiedClassName allows
a package prefix.  Maybe it needs a different name.

The testing looks adequate, but you might consider using
a little combinatorial code to generate bad and good class
names.  Such code could be shared between the unit
tests that test x.y names and Lx/y; names.

Something like:

char goodsep = '.' or '/';
ArrayList badnames, goodnames; …
goodnames.addAll(goodunqnames);
badnames.addAll(badunqnames);
for (int i = 0; i < 2; i++) { 
for (String pkg : badpkgnames) {
…prepend pkg+goodsep to each badnames and goodnames and add to badnames...
}
for (char sep : "./;[".toCharArray()) {
for (String pkg : goodpkgnames) {
if (sep == goodsep)
  …prepend pkg+sep to each goodnames and add to goodnames…
else
…prepend pkg+sep to each goodnames and add to badnames…
}
…prepend sep to each goodnames and add to badnames…
…appendpend sep to each goodnames and add to badnames…
}
return List.of(badnames, goodnames); // return for use in unit test

> On Jan 7, 2019, at 10:17 AM, Vicente Romero  wrote:
> 
> I have updated the webrev after additional feedback from the TCK tester 
> please check last version at [1]
> 
> Thanks,
> Vicente
> 
> [1] http://cr.openjdk.java.net/~vromero/8215510/webrev.01
> 
> 
> On 1/3/19 12:21 PM, Vicente Romero wrote:
>> Please review the fix for bug [1] at [2]. Basically the constants API 
>> introduced as part of JEP-334 [3] were accepting descriptors and class names 
>> not allowed by the spec. This implementation fixes several issues found by 
>> TCK tests on JEP-334,
>> 
>> Thanks,
>> Vicente
>> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8215510
>> [2] http://cr.openjdk.java.net/~vromero/8215510/webrev.00/
>> [3] https://bugs.openjdk.java.net/browse/JDK-8203252
> 



Re: RFR: JDK-8215510: j.l.c.ClassDesc is accepting descriptors not allowed by the spec

2019-01-09 Thread John Rose
On Jan 9, 2019, at 2:04 PM, John Rose  wrote:
> 
> you might consider using
> a little combinatorial code to generate bad and good class
> names

P.S. To motivate this suggestion a bit more:  I found no
problem with your manually-written test vectors of bad
and good names, but I also found it difficult to be confident
that they covered the ground adequately.  Combinatorial
logic for generating test vectors is often overkill, but it is
easier to be confident about.  That in turn makes us more
confident that the actual code itself is free of bugs.

Also, when I use such tactics in my own test development,
I often find more bugs in my code, than with my first cut
tests, which are small ad hoc test vectors.  I won't promise
the same for you, but it's a possibility.


RFR 8216407 : java.util.UUID.fromString accepts input that does not match expected format

2019-01-09 Thread Ivan Gerasimov

Hello!

String representation of UUID should conform to RFC4122 
, i.e. each its part has to be of 
the fixed size.


Unfortunately, the UUID.fromString() method does not keep up to this 
requirement:

- First, it permits the leading zeroes of any part to be omitted;
- Second, it permits some of the UUID parts to be larger then allowed.  
In such a case, the value is effectively clipped with & 0x...
While some existing application may depend on the former case -- i.e. be 
able to parse UUID with stripped leading zeroes, the later case is 
likely to be an error.


In the past, the check on the input has already been strengthened with 
JDK-8006627 .


I propose we go further and make UUID.fromString() to reject such string 
representations that contain too large individual parts.


If people agree on the proposal, I'll file CSR to fix the change of 
behavior.


BUGURL: https://bugs.openjdk.java.net/browse/JDK-8216407
WEBREV: http://cr.openjdk.java.net/~igerasim/8216407/00/webrev/

Thanks in advance!

--
With kind regards,
Ivan Gerasimov



Re: [RFR] 8216362: Incorrect jar file error message when there is an invalid manifest

2019-01-09 Thread Philipp Kunz


Better late than too late: Would it be an option to catch the exception
in JarFile.getManifestFromReference and throw another one with the name
and the caught exception as a cause? Because this is the only code that
calls that particular constructor that I know of. JarVerifier could be
a candidate or SignatureFileVerifier. And JarSigner but JarSigner could
not invoke it like it is now because not visible. I'm not aware of
future plans but if JarFile.getManifestFromReference is the only code
using jar file names in exception messages, we might as well leave
Manifest alone.
Most minor, where is throw new IOException("invalid manifest format(" i
guess there should be a space before the opening bracket at the end of
the string on line 321 to be consistent with the other messages.

On Wed, 2019-01-09 at 15:42 -0500, Lance Andersen wrote:
> Here is the webrev for the changes:
>  http://cr.openjdk.java.net/~lancea/8216362/webrev.00/index.html
> 


Re: [RFR] 8216362: Incorrect jar file error message when there is an invalid manifest

2019-01-09 Thread Lance Andersen
> On Jan 9, 2019, at 6:34 PM, Philipp Kunz  wrote:
> 
> 
> Better late than too late: Would it be an option to catch the exception in 
> JarFile.getManifestFromReference and throw another one with the name and the 
> caught exception as a cause? Because this is the only code that calls that 
> particular constructor that I know of. JarVerifier could be a candidate or 
> SignatureFileVerifier. And JarSigner but JarSigner could not invoke it like 
> it is now because not visible. I'm not aware of future plans but if 
> JarFile.getManifestFromReference is the only code using jar file names in 
> exception messages, we might as well leave Manifest alone.

I would prefer to leave it as it is for now.
> 
> Most minor, where is throw new IOException("invalid manifest format(" i guess 
> there should be a space before the opening bracket at the end of the string 
> on line 321 to be consistent with the other messages.

I will fix before I push
> 
> 
> On Wed, 2019-01-09 at 15:42 -0500, Lance Andersen wrote:
>> Here is the webrev for the changes:
>> 
>>  http://cr.openjdk.java.net/~lancea/8216362/webrev.00/index.html 
>> 
>> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 





Re: RFR 8216407 : java.util.UUID.fromString accepts input that does not match expected format

2019-01-09 Thread Joe Darcy

Hi Ivan,

How does this bug relate to the recent discussion of "JDK-8165199: 
UUID.fromString(str) compliance checking?":


http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057470.html

Cheers,

-Joe

On 1/9/2019 3:23 PM, Ivan Gerasimov wrote:

Hello!

String representation of UUID should conform to RFC4122 
, i.e. each its part has to be of 
the fixed size.


Unfortunately, the UUID.fromString() method does not keep up to this 
requirement:

- First, it permits the leading zeroes of any part to be omitted;
- Second, it permits some of the UUID parts to be larger then 
allowed.  In such a case, the value is effectively clipped with & 
0x...
While some existing application may depend on the former case -- i.e. 
be able to parse UUID with stripped leading zeroes, the later case is 
likely to be an error.


In the past, the check on the input has already been strengthened with 
JDK-8006627 .


I propose we go further and make UUID.fromString() to reject such 
string representations that contain too large individual parts.


If people agree on the proposal, I'll file CSR to fix the change of 
behavior.


BUGURL: https://bugs.openjdk.java.net/browse/JDK-8216407
WEBREV: http://cr.openjdk.java.net/~igerasim/8216407/00/webrev/

Thanks in advance!



Re: RFR 8216407 : java.util.UUID.fromString accepts input that does not match expected format

2019-01-09 Thread Ivan Gerasimov

Hi Joe!

From what I see, the discussion was about possibility to only accept 
input strings with precisely sized parts, i.e. that matches
"\p{XDigit}{8}- \p{XDigit}{4}- \p{XDigit}{4}- \p{XDigit}{4}- 
\p{XDigit}{12}".


And I'm proposing to only reject input with too large individual parts, 
i.e. accept any input that matches
" \p{XDigit}{1,8}- \p{XDigit}{1,4}- \p{XDigit}{1,4}- \p{XDigit}{1,4}- 
\p{XDigit}{1,12}".


Currently, if one part is too large, we already "clip" it, using only 
lower bits, so behavior looks buggy.


Instead, it seems better to report the issue throwing an exception.

With kind regards,
Ivan

On 1/9/19 11:08 PM, Joe Darcy wrote:

Hi Ivan,

How does this bug relate to the recent discussion of "JDK-8165199: 
UUID.fromString(str) compliance checking?":


http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057470.html 



Cheers,

-Joe

On 1/9/2019 3:23 PM, Ivan Gerasimov wrote:

Hello!

String representation of UUID should conform to RFC4122 
, i.e. each its part has to be 
of the fixed size.


Unfortunately, the UUID.fromString() method does not keep up to this 
requirement:

- First, it permits the leading zeroes of any part to be omitted;
- Second, it permits some of the UUID parts to be larger then 
allowed.  In such a case, the value is effectively clipped with & 
0x...
While some existing application may depend on the former case -- i.e. 
be able to parse UUID with stripped leading zeroes, the later case is 
likely to be an error.


In the past, the check on the input has already been strengthened 
with JDK-8006627 .


I propose we go further and make UUID.fromString() to reject such 
string representations that contain too large individual parts.


If people agree on the proposal, I'll file CSR to fix the change of 
behavior.


BUGURL: https://bugs.openjdk.java.net/browse/JDK-8216407
WEBREV: http://cr.openjdk.java.net/~igerasim/8216407/00/webrev/

Thanks in advance!





--
With kind regards,
Ivan Gerasimov



[12] JDK-8211841 [testbug] sun/nio/cs/OLD/TestIBMDB.java does not compile (aix)

2019-01-09 Thread Ichiroh Takiguchi

Hello.
Could you review the fix ?
And please push this to jdk12 if possible.

Bug:https://bugs.openjdk.java.net/browse/JDK-8211841
Change: http://cr.openjdk.java.net/~itakiguchi/8211841/webrev.00/

I'd like to obtain a sponsor for this issue.

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.