Re: 答复: Fix for https://bugs.openjdk.java.net/browse/JDK-8017779

[Kurchi Subhra Hazra]

Thanks Tristan for following up with this!

-Kurchi


> On Oct 22, 2013, at 5:28 AM, Chris Hegarty  wrote:
> 
> Thanks Tristan.
> 
> Given your clarifications, I will push this change for you.
> 
> -Chris.
> 
>> On 22/10/2013 13:13, Tristan Yan wrote:
>> Hi Chris
>> Have you verified Kurchi's changes, tested, reviewed
>> Yes, I did, I tested Kurchi's fix in one of our failed 
>> machine(sc11136394.us), with 1000 times run, I still can see one time 
>> failure.
>> 
>> Have you modified Kurchi's changes from her original review request
>> Yes, that's exact what I did, I stole her code and change and moved line 
>> 163, 192, 193 and line 322. I also verify the final fix in our failed 
>> machine(sc11136394.us) with 1000 times run, it all passed.
>> Thank you very much
>> Tristan
>> 
>> -邮件原件-
>> 发件人: Chris Hegarty
>> 发送时间: Tuesday, October 22, 2013 5:47 PM
>> 收件人: Tristan Yan
>> 抄送: net-dev@openjdk.java.net
>> 主题: Re: Fix for https://bugs.openjdk.java.net/browse/JDK-8017779
>> 
>> Hi Tristan,
>> 
>> I agree with you, option 2 is probably better.
>> 
>> Have you verified Kurchi's changes, tested, reviewed, etc?
>> Have you modified Kurchi's changes from her original review request?
>> 
>> I can sponsor this change.
>> 
>> Thanks,
>> -Chris.
>> 
>>> On 22/10/2013 07:22, Tristan Yan wrote:
>>> Hi Everyone
>>> 
>>> I have a fix for https://bugs.openjdk.java.net/browse/JDK-8022211,
>>> could you review it.
>>> 
>>> Since we have Kurchi's code change out for review for re-writing this
>>> test to use the new HTTP Server API. We have 2 option here 1. Just
>>> fixing the bug with move setCondition around.
>>> 2. Adopting Kurchi's code change and my fix together as a whole fix.
>>> I propose we're using second way, the reason is Kruchi was using
>>> modern API of JDK, it's less error-prone and make test shorter, I'd
>>> like to shameless steal her fix as part of my fix.
>>> 
>>> http://cr.openjdk.java.net/~pzhang/Tristan/8017779/webrev/
>>> 
>>> /Tristan Yan(Haibo Yan)/
>>> 
>>> /Office : 8610-61066212/
>>> 
>>> /Fax : 8610-61065441/
>>> 
>>> /Cell : 86-18610696822/
>>> 
>>> //
>>> 
>>> /2F, Building No. 24, Zhongguancun Software Park/
>>> 
>>> /Haidian District Beijing
>>> >> :P6_CITY:Beijing>
>>> , 100193/
>>> 
>>> oracle
>>> 

Re: Review problemlist.txt

[Kurchi Subhra Hazra]

That is fine with me.

Thanks,
Kurchi

On 10/19/11 8:48 AM, Chris Hegarty wrote:

Michael, Kurchi,

java/net/DatagramSocket/ChangingAddress.java was added recently in 
JDK8, see CR 7084030. Looking at it again I'm not sure if the test 
makes sense, or at least we would have to loosen its restrictions so 
much that it wouldn't be testing anything useful ( or the crooks of 
the original change it was added for).


We have a very similar test in nio, 
java/nio/channels/DatagramChannel/ChangingAddress.java, which already 
covers similar functionality.


How about we just remove it?

-Chris.

On 19/10/2011 15:45, Michael McMahon wrote:

Hi,

We have a few test failures on the nightly test run.
So, I'd like to move them on to the problem list pending
investigation.

http://cr.openjdk.java.net/~michaelm/7102665/webrev.1/

Thanks,
Michael

Re: Review problemlist.txt

[Kurchi Subhra Hazra]

That is fine with me. Can I remove the test as a part of fixing 7102704?

Thanks,
Kurchi

On 10/19/11 8:48 AM, Chris Hegarty wrote:

Michael, Kurchi,

java/net/DatagramSocket/ChangingAddress.java was added recently in 
JDK8, see CR 7084030. Looking at it again I'm not sure if the test 
makes sense, or at least we would have to loosen its restrictions so 
much that it wouldn't be testing anything useful ( or the crooks of 
the original change it was added for).


We have a very similar test in nio, 
java/nio/channels/DatagramChannel/ChangingAddress.java, which already 
covers similar functionality.


How about we just remove it?

-Chris.

On 19/10/2011 15:45, Michael McMahon wrote:

Hi,

We have a few test failures on the nightly test run.
So, I'd like to move them on to the problem list pending
investigation.

http://cr.openjdk.java.net/~michaelm/7102665/webrev.1/

Thanks,
Michael

hg: jdk8/tl/jdk: 7146763: Warnings cleanup in the sun.rmi and related packages

[kurchi . subhra . hazra]

Changeset: 8f61ac5986ee
Author:khazra
Date:  2012-03-02 13:48 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8f61ac5986ee

7146763: Warnings cleanup in the sun.rmi and related packages
Summary: Cleanup warnings and use jkd7 features in sun.rmi.*
Reviewed-by: smarks, chegar, forax, dmocek

! make/sun/rmi/cgi/Makefile
! make/sun/rmi/registry/Makefile
! make/sun/rmi/rmi/Makefile
! make/sun/rmi/rmid/Makefile
! src/share/classes/com/sun/rmi/rmid/ExecOptionPermission.java
! src/share/classes/com/sun/rmi/rmid/ExecPermission.java
! src/share/classes/sun/rmi/log/ReliableLog.java
! src/share/classes/sun/rmi/registry/RegistryImpl.java
! src/share/classes/sun/rmi/rmic/BatchEnvironment.java
! src/share/classes/sun/rmi/rmic/Main.java
! src/share/classes/sun/rmi/rmic/RMIGenerator.java
! src/share/classes/sun/rmi/rmic/newrmic/Main.java
! src/share/classes/sun/rmi/rmic/newrmic/Resources.java
! src/share/classes/sun/rmi/server/ActivatableRef.java
! src/share/classes/sun/rmi/server/Activation.java
! src/share/classes/sun/rmi/server/ActivationGroupImpl.java
! src/share/classes/sun/rmi/server/LoaderHandler.java
! src/share/classes/sun/rmi/server/MarshalInputStream.java
! src/share/classes/sun/rmi/server/UnicastRef.java
! src/share/classes/sun/rmi/server/UnicastRef2.java
! src/share/classes/sun/rmi/server/UnicastServerRef.java
! src/share/classes/sun/rmi/server/Util.java
! src/share/classes/sun/rmi/server/WeakClassHashMap.java
! src/share/classes/sun/rmi/transport/ConnectionInputStream.java
! src/share/classes/sun/rmi/transport/DGCAckHandler.java
! src/share/classes/sun/rmi/transport/DGCClient.java
! src/share/classes/sun/rmi/transport/DGCImpl.java
! src/share/classes/sun/rmi/transport/ObjectTable.java
! src/share/classes/sun/rmi/transport/StreamRemoteCall.java
! src/share/classes/sun/rmi/transport/Target.java
! src/share/classes/sun/rmi/transport/Transport.java
! src/share/classes/sun/rmi/transport/WeakRef.java
! src/share/classes/sun/rmi/transport/proxy/CGIHandler.java
! src/share/classes/sun/rmi/transport/proxy/HttpInputStream.java
! src/share/classes/sun/rmi/transport/proxy/HttpSendSocket.java
! src/share/classes/sun/rmi/transport/proxy/RMIMasterSocketFactory.java
! src/share/classes/sun/rmi/transport/tcp/ConnectionMultiplexer.java
! src/share/classes/sun/rmi/transport/tcp/TCPChannel.java
! src/share/classes/sun/rmi/transport/tcp/TCPEndpoint.java
! src/share/classes/sun/rmi/transport/tcp/TCPTransport.java

hg: jdk8/tl/jdk: 7151348: Build breaks due to warning clean up in sun.rmi.*(7146763)

[kurchi . subhra . hazra]

Changeset: ce6b852bf4e2
Author:khazra
Date:  2012-03-05 17:38 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ce6b852bf4e2

7151348: Build breaks due to warning clean up in sun.rmi.*(7146763)
Summary: Undo changes to sun/rmi/rmic/* commited as fix for 7146763
Reviewed-by: smarks

! src/share/classes/sun/rmi/rmic/BatchEnvironment.java
! src/share/classes/sun/rmi/rmic/Main.java
! src/share/classes/sun/rmi/rmic/RMIGenerator.java
! src/share/classes/sun/rmi/rmic/newrmic/Main.java
! src/share/classes/sun/rmi/rmic/newrmic/Resources.java

hg: jdk8/tl/jdk: 7045655: An empty InMemoryCookieStore should not return true for removeAll

[kurchi . subhra . hazra]

Changeset: 3bfebedb549f
Author:khazra
Date:  2012-03-15 13:21 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3bfebedb549f

7045655: An empty InMemoryCookieStore should not return true for removeAll
Summary: CookieStore.removeAll() should return false for an empty CookieStore
Reviewed-by: chegar

! src/share/classes/java/net/InMemoryCookieStore.java
! test/java/net/CookieHandler/NullUriCookieTest.java

hg: jdk8/tl/jdk: 7152007: Fix warnings in sun/rmi/rmic

[kurchi . subhra . hazra]

Changeset: 337d4570b8d6
Author:khazra
Date:  2012-03-16 11:52 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/337d4570b8d6

7152007: Fix warnings in sun/rmi/rmic
Summary: Minor code changes to remove warnings in sun/rmi/rmic
Reviewed-by: chegar, smarks

! src/share/classes/sun/rmi/rmic/BatchEnvironment.java
! src/share/classes/sun/rmi/rmic/Main.java
! src/share/classes/sun/rmi/rmic/RMIGenerator.java
! src/share/classes/sun/rmi/rmic/newrmic/Main.java
! src/share/classes/sun/rmi/rmic/newrmic/Resources.java

Re: Code Review Request: 7152856: TEST_BUG: sun/net/www/protocol/jar/B4957695.java failing on Windows

[Kurchi Subhra Hazra]

I think the HTTP spec needs an http server to handle LF gracefully, 
although it expects a CRLF, and that is how this is working.

It is a small change, I will anyway correct it.

- Kurchi


On 4/17/12 3:24 AM, Chris Hegarty wrote:

On 16/04/12 22:18, Kurchi Hazra wrote:

Hi,

Thanks for the reviews. Here is an updated webrev:
http://cr.openjdk.java.net/~khazra/7152856/webrev.01


The updated webrev looks ok, but the "canned" HTTP response looks funny.

Each HTTP header must be followed by a CRLF ( '\r\n' ), and the end of 
the headers ( just before the response body ) marked by CRLF CRLF. Is 
OutputStreamWriter somehow appending a new line?


Sorry, I think at one time I pointed you to another test that may not 
have been sending a valid HTTP response.


-Chris.



Thanks,
Kurchi


On 4/15/2012 12:35 AM, Chris Hegarty wrote:

On 14/04/12 16:53, Alan Bateman wrote:

On 13/04/2012 17:59, Kurchi Hazra wrote:

Hi,

This test was failing on Windows since it was using the HttpServer in
test/sun/net/www/httptest. The HttpServer implementation
there is buggy and does not close the connection properly, resulting
in the test hanging. We therefore write our own server, which sends
back
10 bytes less than what the client expects, and see if the client
raises an IOException.

Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7152856
Webrev: http://cr.openjdk.java.net/~khazra/7152856/webrev.00

Thanks,
Kurchi

Thanks for sorting out this test. A couple of comments:

- I don't think the @run is right as samevm or agentvm is specified to
jtreg rather than on specific tests (it is possible to add /othervm to
force a test to run in its own VM).

- "Server" might be better than XServer (as X server normally means a
X11 server).

- XServer.srv should be final.

- It looks like the server socket is closed when the test terminates.
Also to ensure that the accepted connection is closed I would suggest
that run be changed to try (Socket s = srv.accept()) { ... }.

Otherwise I think it's okay.


I agree with Alan's comments.

Just to add, no @run tag is needed in this test. The default "@run
main " [1] should be fine, and allow the test be run in the mode
specified by the caller. I think this is best where possible.

"If no @run tags are present in a defining file, a default is assumed
depending upon the file's filename extension. For a ".java" file,
"@run main " is assumed, where  is the name of the file
without the ".java" suffix. For a ".sh" file, "@run shell "
is assumed. For an ".html" file, "@run applet " is assumed."

-Chris.

[1] http://openjdk.java.net/jtreg/tag-spec.txt



-Alan.

hg: jdk8/tl/jdk: 7152856: TEST_BUG: sun/net/www/protocol/jar/B4957695.java failing on Windows

[kurchi . subhra . hazra]

Changeset: 31c15e2f51ba
Author:khazra
Date:  2012-04-17 11:59 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/31c15e2f51ba

7152856: TEST_BUG: sun/net/www/protocol/jar/B4957695.java failing on Windows
Summary: Remove usage of HTTP Server at test/sun/net/www/httptest
Reviewed-by: chegar, alanb

! test/sun/net/www/protocol/jar/B4957695.java

hg: jdk8/tl/jdk: 7157893: Warnings Cleanup in java.util.*

[kurchi . subhra . hazra]

Changeset: 1757f049e8c0
Author:khazra
Date:  2012-04-17 12:21 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1757f049e8c0

7157893: Warnings Cleanup in java.util.*
Summary: Minor code changes to cleanup warnings in java.util.*
Reviewed-by: mduigou, naoto, smarks
Contributed-by: Remi Forax 

! src/share/classes/java/util/AbstractCollection.java
! src/share/classes/java/util/AbstractList.java
! src/share/classes/java/util/AbstractMap.java
! src/share/classes/java/util/AbstractSet.java
! src/share/classes/java/util/ArrayDeque.java
! src/share/classes/java/util/ArrayList.java
! src/share/classes/java/util/Arrays.java
! src/share/classes/java/util/Calendar.java
! src/share/classes/java/util/Collections.java
! src/share/classes/java/util/ComparableTimSort.java
! src/share/classes/java/util/Currency.java
! src/share/classes/java/util/EnumMap.java
! src/share/classes/java/util/EnumSet.java
! src/share/classes/java/util/HashMap.java
! src/share/classes/java/util/HashSet.java
! src/share/classes/java/util/Hashtable.java
! src/share/classes/java/util/IdentityHashMap.java
! src/share/classes/java/util/IllegalFormatConversionException.java
! src/share/classes/java/util/JumboEnumSet.java
! src/share/classes/java/util/LinkedHashMap.java
! src/share/classes/java/util/Observable.java
! src/share/classes/java/util/PriorityQueue.java
! src/share/classes/java/util/Properties.java
! src/share/classes/java/util/PropertyPermission.java
! src/share/classes/java/util/RegularEnumSet.java
! src/share/classes/java/util/ResourceBundle.java
! src/share/classes/java/util/ServiceLoader.java
! src/share/classes/java/util/TimeZone.java
! src/share/classes/java/util/TreeMap.java
! src/share/classes/java/util/TreeSet.java
! src/share/classes/java/util/WeakHashMap.java

hg: jdk8/tl/jdk: 7162385: TEST_BUG: sun/net/www/protocol/jar/B4957695.java failing again

[kurchi . subhra . hazra]

Changeset: bc51d0569ccd
Author:khazra
Date:  2012-04-19 13:26 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bc51d0569ccd

7162385: TEST_BUG: sun/net/www/protocol/jar/B4957695.java failing again
Summary: Enable finding "foo1.jar"
Reviewed-by: chegar

! test/sun/net/www/protocol/jar/B4957695.java

hg: jdk8/tl/jdk: 7158636: InterfaceAddress.getBroadcast() returns invalid broadcast address on WLAN

[kurchi . subhra . hazra]

Changeset: 715f50872ae7
Author:khazra
Date:  2012-04-19 18:11 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/715f50872ae7

7158636: InterfaceAddress.getBroadcast() returns invalid broadcast address on 
WLAN
Summary: Update Windows native code to infer WLAN interface type in Windows 
Vista and later
Reviewed-by: chegar, alanb

! src/windows/native/java/net/NetworkInterface.c
! src/windows/native/java/net/NetworkInterface.h

hg: jdk8/tl/jdk: 7144274: [macosx] Default IPv6 multicast interface is not being set when calling MulticastSocket.joinGroup()

[kurchi . subhra . hazra]

Changeset: fcdbd1f34309
Author:khazra
Date:  2012-04-24 14:59 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/fcdbd1f34309

7144274: [macosx] Default IPv6 multicast interface is not being set when 
calling MulticastSocket.joinGroup()
Summary: Get default interface for Mac OS X when interface is not set
Reviewed-by: chegar

! src/solaris/native/java/net/PlainDatagramSocketImpl.c
! src/solaris/native/java/net/net_util_md.c

hg: jdk8/tl/jdk: 7160242: (prefs) Preferences.remove(null) does not throw NPE [macosx]

[kurchi . subhra . hazra]

Changeset: 3e398b549cea
Author:khazra
Date:  2012-04-25 12:31 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3e398b549cea

7160242: (prefs) Preferences.remove(null) does not throw NPE [macosx]
Summary: Insert null check of argument in remove()'s implementation
Reviewed-by: forax, chegar, alanb

! src/macosx/classes/java/util/prefs/MacOSXPreferences.java
+ test/java/util/prefs/RemoveNullKeyCheck.java

hg: jdk8/tl/jdk: 7118100: (prefs) Inconsistency when using system and user preference on OSX Lion

[kurchi . subhra . hazra]

Changeset: 108a02a57b75
Author:khazra
Date:  2012-04-26 12:04 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/108a02a57b75

7118100: (prefs) Inconsistency when using system and user preference on OSX Lion
Summary: Enable user to read/write preferences to persistent storage
Reviewed-by: alanb

! src/macosx/classes/java/util/prefs/MacOSXPreferences.java
! src/macosx/classes/java/util/prefs/MacOSXPreferencesFile.java

hg: jdk8/tl/jdk: 7165118: (prefs) AbstractPreferences.remove(null) does not throw NPE

[kurchi . subhra . hazra]

Changeset: 59121a4c71c6
Author:khazra
Date:  2012-05-09 11:14 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/59121a4c71c6

7165118: (prefs) AbstractPreferences.remove(null) does not throw NPE
Summary: Insert null argument check in AbstractPreferences.remove()
Reviewed-by: dholmes, chegar, alanb

! src/share/classes/java/util/prefs/AbstractPreferences.java
! test/java/util/prefs/RemoveNullKeyCheck.java

hg: jdk8/tl/jdk: 7096436: (sc) SocketChannel.connect fails on Windows 8 when channel configured non-blocking

[kurchi . subhra . hazra]

Changeset: 5152c832745a
Author:khazra
Date:  2012-05-09 16:55 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5152c832745a

7096436: (sc) SocketChannel.connect fails on Windows 8 when channel configured 
non-blocking
Summary: Set localAddress only when connection is established
Reviewed-by: alanb

! src/share/classes/sun/nio/ch/SocketChannelImpl.java

hg: jdk8/tl/jdk: 7164636: (prefs) Cleanup src/macosx/classes/java/util/prefs

[kurchi . subhra . hazra]

Changeset: 9a18e318f95a
Author:khazra
Date:  2012-05-15 11:51 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9a18e318f95a

7164636: (prefs) Cleanup src/macosx/classes/java/util/prefs
Summary: Remove rawtype usages and other code cleanup
Reviewed-by: chegar, briangoetz

! src/macosx/classes/java/util/prefs/MacOSXPreferences.java
! src/macosx/classes/java/util/prefs/MacOSXPreferencesFactory.java
! src/macosx/classes/java/util/prefs/MacOSXPreferencesFile.java

hg: jdk8/tl/jdk: 7170169: (props) System.getProperty("os.name") should return "Windows 8" when run on Windows 8

[kurchi . subhra . hazra]

Changeset: 0c3d9050c918
Author:khazra
Date:  2012-05-23 10:41 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0c3d9050c918

7170169: (props) System.getProperty("os.name") should return "Windows 8" when 
run on Windows 8
Summary: Enable Windows Version 6.2 to be recognized as Windows 8
Reviewed-by: darcy, dholmes, alanb, chegar

! src/windows/native/java/lang/java_props_md.c

hg: jdk8/tl/jdk: 7171591: getDefaultScopeID() in src/solaris/native/java/net/net_util_md.c should return a value

[kurchi . subhra . hazra]

Changeset: eb441933f6fe
Author:khazra
Date:  2012-05-29 13:16 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/eb441933f6fe

7171591: getDefaultScopeID() in src/solaris/native/java/net/net_util_md.c 
should return a value
Summary: Use CHECK_NULL_RETURN instead of CHECK_NULL
Reviewed-by: alanb

! src/solaris/native/java/net/net_util_md.c

hg: jdk8/tl/jdk: 7173645: (props) System.getProperty("os.name") should return "Windows Server 2012" for Windows Server 2012

[kurchi . subhra . hazra]

Changeset: af313ded4ffb
Author:khazra
Date:  2012-06-06 11:37 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/af313ded4ffb

7173645: (props) System.getProperty("os.name") should return "Windows Server 
2012" for Windows Server 2012
Summary: Enable Windows Server 2012 to be recognized as "os.name"
Reviewed-by: alanb, dholmes, chegar

! src/windows/native/java/lang/java_props_md.c

Code Review Request: 7154502: IAE: Illegal character(s) in message header field from HttpURLConnection.checkMessageHeader

[Kurchi Subhra Hazra]

Hi,

   Currently HttpURLConnection.setRequestProperty() does not
document that it can throw an IllegalArgumentException. However, this method 
does
throw an IAE if the key or value contains illegal characters. The same applies
for HttpURLConnection.addRequestProperty(), and this webrev tries to address 
this
issue.

Bug: http://bugs.sun.com/view_bug.do?bug_id=7154502
Webrev: http://cr.openjdk.java.net/~khazra/7154502/webrev.00/

Thanks,
Kurchi

hg: jdk8/tl/jdk: 7160252: (prefs) NodeAddedEvent was not delivered when new node add when new Node

[kurchi . subhra . hazra]

Changeset: e9461aeff91f
Author:khazra
Date:  2012-07-13 16:02 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e9461aeff91f

7160252: (prefs) NodeAddedEvent was not delivered when new node add when new 
Node
Summary: Change native code to convey to Java code whether a new node was added
Reviewed-by: alanb, chegar

! src/macosx/classes/java/util/prefs/MacOSXPreferences.java
! src/macosx/classes/java/util/prefs/MacOSXPreferencesFile.java
! src/macosx/native/java/util/MacOSXPreferencesFile.m
+ test/java/util/prefs/AddNodeChangeListener.java

hg: jdk8/tl/jdk: 7177045: Rework the TestProviderLeak.java regression test, it is too fragile to low memory errors.

[kurchi . subhra . hazra]

Changeset: 1469be7182b4
Author:khazra
Date:  2012-07-16 16:30 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1469be7182b4

7177045: Rework the TestProviderLeak.java regression test, it is too fragile to 
low memory errors.
Summary: Increase Xmx to 20 MB and add mechanisms to eat up most of the JVM 
free memory.
Reviewed-by: wetmore
Contributed-by: dan...@oracle.com

! test/com/sun/crypto/provider/KeyFactory/TestProviderLeak.java

Re: CR: 7183292: HttpURLConnection.getHeaderFields() throws IllegalArgumentException: Illegal cookie name

[Kurchi Subhra Hazra]

I have read the sections dealing with cookie-name in 6265, and these 
changes look good to me.


- Kurchi

On 7/17/12 7:32 AM, Michael McMahon wrote:


Thanks for reviewing this Chris. On the question of whether $ should 
be allowed
in cookie names, it appears like that restriction has been removed 
from RFC 6265,
which is evidently a fairly comprehensive description of actual cookie 
usage on the web.
So, maybe we should just leave that out as well - assuming that it is 
being used in places

(albeit in contravention of the older RFC). What do you think?

- Michael

On 17/07/2012 14:18, Chris Hegarty wrote:

On 17/07/2012 10:17, Michael McMahon wrote:

Hi,

Could I get the following change reviewed please?

http://cr.openjdk.java.net/~michaelm/7183292/webrev.1/

Since 7u4, we are parsing all incoming cookies via the HttpCookie 
class.

This class has had a restriction on cookie names that is causing this
problem
and which is not required by any of the cookie specifications, as 
far as

I can see,
(rfc 2965, and 6265 which obsoletes 2965).


Right, this is my reading of the RFC's also. In fact, RFC 2965 
explicitly states that "the NAME of a cookie MAY be the same as one 
of the attributes in this specification".



The restriction was that cookie names could not be the same (case
insensitively)
as any of the attribute names (eg. Domain). So, the change is to remove
the restriction.


Yes, this makes sense to me.

One comment on the webrev is that isReserved also enforces that the 
name cannot start with a '$', from 2965: "NAMEs that begin with $ are 
reserved and MUST NOT be used by applications." I think you may need 
to minimally reintroduce this. Otherwise, the changes look good to me.


-Chris.



Thanks,
Michael

hg: jdk8/tl/jdk: 7185051: Remove TestProviderLeak.java from the ProblemList

[kurchi . subhra . hazra]

Changeset: 773474da570b
Author:khazra
Date:  2012-07-18 15:19 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/773474da570b

7185051: Remove TestProviderLeak.java from the ProblemList
Summary: Remove TestProviderLeak.java from jdk test problem list.
Reviewed-by: khazra
Contributed-by: dan...@oracle.com

! test/ProblemList.txt

hg: jdk8/tl/jdk: 7184287: (prefs) BackingStoreException when calling flush on root node[macosx]

[kurchi . subhra . hazra]

Changeset: 74ceda3a98a0
Author:khazra
Date:  2012-07-24 13:38 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/74ceda3a98a0

7184287: (prefs) BackingStoreException when calling flush on root node[macosx]
Summary: Change implementation to enable user without administrative privileges 
to call flush
Reviewed-by: alanb

! src/macosx/classes/java/util/prefs/MacOSXPreferences.java
! src/macosx/classes/java/util/prefs/MacOSXPreferencesFile.java
! test/ProblemList.txt

Re: Http client API

[Kurchi Subhra Hazra]

- HttpClient createClient() -- is this the same as 
HttpClientProvider.provider().createAsynchronousHttpClient()?

yes, and that needs to be documented.
Just on this one, the method name createAsynchronousHttpClient() looks 
like a remnant of our previous API iterations, and I think this

should just be renamed to createHttpClient() or createClient().

- Kurchi

Re: Http client API

[Kurchi Subhra Hazra]

Just as a reminder, for reading the response, we had originally decided to
simply terminate whatever bytebuffer/byte arrays we have with a -1 to 
indicate
EOF. But if returning the number of bytes read results in cleaner code, 
maybe we should do this.




HttpResponse::

- getBodyAsBytes(byte[], int) - I would rather the return the number 
of bytes put into the buffer.
yes. Actually the number of bytes read needs to be returned somewhere. 
It should return

an int or long
- getBodyAsBytes(byte[], int) - Why would I ever want a max size less 
than the size of my byte buffer?
I originally wanted to implement the no-arg getBodyAsBytes() using the 
other method.

Maybe it's better to replace them both with:

long getBodyAsBytes(byte[] buffer) - buffer can't be null, use entire 
buffer, return # bytes read
byte[] getBodyAsBytes() - size limited by 
HttpClient.getResponseBodyBufferLimit()


- getBodyAsBytes -- Document handling for content-length>  
Integer.MAX_VALUE
dealt with implicitly from above change and the response body buffer 
limit itself.
- getBodyAsByteBuffers -- handling for not enough space in the array 
needs to be documented. Unused entries nulled out?
situation would only arise through ByteBuffer allocation failure, and 
presumably OOM exception would be thrown.

Is it necessary to document this?

- Perhaps maxlength ->  maxBytes to be more clear?

- getBodyAsByteBufferSource -- if the same iterator is always 
returned why not return the iterator rather than an iterable?

The idea was to allow for use of the foreach convenience. eg

Iterable  buffers = response.getBodyAsByteBufferSource();
for (ByteBuffer buffer : buffers) {
// handle data in buffer
}


This raises the same concern then about the Iterable being 
non-restartable, which in this case
it clearly can't be. This is a "one-shot" streaming API. I notice that 
the documentation for

Iterable is rather terse and doesn't say whether
this usage is encouraged or discouraged. So, I'm not sure now. The 
question is if programmers would
really be confused by the fact that the Iterable can only be used to 
return one Iterator instance.



HttpConnectionCache.CachedConnection::
- I would expect this to be abstract and that client providers would 
extend.


- getCache() to return the client provider.


On Aug 7 2012, at 16:09 , Michael McMahon wrote:


Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.

Thanks
Michael McMahon.

hg: jdk8/tl/jdk: 7168172: (fs) Files.isReadable slow on Windows

[kurchi . subhra . hazra]

Changeset: bd91a601265c
Author:khazra
Date:  2012-08-24 11:48 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bd91a601265c

7168172: (fs) Files.isReadable slow on Windows
Summary: Remove DACL checking for read access, also reviewed by 
ulf.zi...@cosoco.de, zhong.j...@gmail.com
Reviewed-by: alanb

! src/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java

Re: Review request for bug number: 6354758, aka "rename old test HttpServer classes"

[Kurchi Subhra Hazra]

Looks good John. Any particular reason that you renamed HttpServer to 
TestHttpServer

but HttpsServer to SimpleHttpsServer?

Thanks,
- Kurchi

On 9/5/12 7:39 AM, John Zavgren wrote:

Greetings:

Please help to review the fix below for bug 6354758:
http://cr.openjdk.java.net/~chegar/6354758/webrev.00/

I changed the name of the class: ./test/sun/net/www/httptest/HttpServer.java 
to: ./test/sun/net/www/httptest/TestHttpServer.java,
to eliminate a possible "name space collision". The server in the test code 
(which is intended to be used ONLY for testing) has the same name as the server that's 
used as an actual Http Server:
./jdk/src/share/classes/com/sun/net/httpserver/HttpServer.java

Thanks,
John Zavgren

hg: jdk8/tl/jdk: 7195933: There is incorrect link to "Info-ZIP Application Note 970311" in doc page of Package java.util.zip

[kurchi . subhra . hazra]

Changeset: e143d8f8e477
Author:dxu
Date:  2012-09-18 14:45 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e143d8f8e477

7195933: There is incorrect link to "Info-ZIP Application Note 970311" in doc 
page of Package java.util.zip
Summary: Correct a java doc link in java.util.zip package page
Reviewed-by: chegar, lancea, sherman
Contributed-by: dan...@oracle.com

! src/share/classes/java/util/zip/package.html

hg: jdk8/tl/jdk: 7198073: (prefs) user prefs not saved [macosx]

[kurchi . subhra . hazra]

Changeset: 3a6b76a468bd
Author:khazra
Date:  2012-10-16 15:23 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3a6b76a468bd

7198073: (prefs) user prefs not saved [macosx]
Summary: Using class member field to get node instead of argument
Reviewed-by: alanb

! src/macosx/classes/java/util/prefs/MacOSXPreferences.java
+ test/java/util/prefs/CheckUserPrefFirst.java
+ test/java/util/prefs/CheckUserPrefLater.java
+ test/java/util/prefs/CheckUserPrefsStorage.sh

hg: jdk8/tl/jdk: 8003518: (prefs) Tests in jdk/test/java/util/prefs should not be run concurrently

[kurchi . subhra . hazra]

Changeset: 0ee09f17361e
Author:khazra
Date:  2012-11-16 12:28 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0ee09f17361e

8003518: (prefs) Tests in jdk/test/java/util/prefs should not be run 
concurrently
Summary: Add java/util/prefs to exclusiveAccess.dirs in TEST.ROOT
Reviewed-by: alanb, mchung

! test/TEST.ROOT

hg: jdk8/tl/jdk: 7197662: (prefs) java/util/prefs/AddNodeChangeListener.java fails by timeout or by "couldn't get file lock"

[kurchi . subhra . hazra]

Changeset: 43d2e02c4098
Author:khazra
Date:  2012-11-30 12:00 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/43d2e02c4098

7197662: (prefs) java/util/prefs/AddNodeChangeListener.java fails by timeout or 
by "couldn't get file lock"
Summary: Set -Djava.util.prefs.userRoot to current working directory of user in 
the prefs tests
Reviewed-by: alanb, chegar, weijun, dxu

! test/java/util/prefs/AddNodeChangeListener.java
! test/java/util/prefs/CheckUserPrefsStorage.sh
! test/java/util/prefs/CommentsInXml.java
! test/java/util/prefs/ConflictInFlush.java
! test/java/util/prefs/ExportNode.java
! test/java/util/prefs/ExportSubtree.java
! test/java/util/prefs/PrefsSpi.sh
! test/java/util/prefs/RemoveNullKeyCheck.java
! test/java/util/prefs/RemoveReadOnlyNode.java
! test/java/util/prefs/RemoveUnregedListener.java

Re: Code Review Request: 7171415: java.net.URI.equals/hashCode not consistent for some URIs

[Kurchi Subhra Hazra]

Thanks a lot for your comments David.

On 1/8/13 6:39 PM, David Schlosnagle wrote:

Hi Kurchi,

I had a couple quick comments:

On line 1756, you might want to allocate the StringBuilder normalizedS
= new StringBuilder(s.length()); to avoid resizing during the
appending.

- I am now wondering if I should directly calculate the hashCode instead of
using a StringBuilder.



On line 1754 and 1759, is it worth restructuring the logic to avoid
performing indexOf twice to find the first '%'? This might be
premature optimization as hopefully the URIs are short and that
indexOf would be replaced with an intrinsic if hot enough.
It is possible to store the value of first occurrence of '%' in a 
variable, but I was

trying to keep the case with no '%' clutter-free.

- Kurchi



Thanks,
Dave

On Tue, Jan 8, 2013 at 8:19 PM, Kurchi Hazra
  wrote:

Hi,

 According to RFC 3986[1], hexadecimal digits encoded by a '%' should be
case-insensitive, for example,%A2 and %a2 should be
considered equal. Although, URI.equals() does take this into consideration,
the implementation of URI.hashCode() does not and
returns different hashcodes for two URIs that are similar in all respects
except for the case of the percent-encoded hexadecimal digits.
This fix attempts to construct a normalized string from the string
representing a component before calculating its hashCode.
I converted to upper case for the normalization(and not lower case) as
required by [1].

 For testing the fix, I added an additional test scenario to an existing
test (jdk/test/java/net/URI/Test.java). While I was there, I also made
minor changes to the test so that it does not produce rawtype and other lint
warnings.

Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7171415
Webrev: http://cr.openjdk.java.net/~khazra/7171415/webrev.00/

URI.compareTo() still suffers from the same problem - I am not sure if it
should be dealt with as a separate bug.

Thanks,
Kurchi

[1] http://tools.ietf.org/html/rfc3986#section-6.2.2.1

Re: Code Review Request: 7171415: java.net.URI.equals/hashCode not consistent for some URIs

[Kurchi Subhra Hazra]

On 1/8/13 6:55 PM, Vitaly Davidovich wrote:


Also, I'm not sure how hot this method is in practice but allocating 
StringBuilder seems a bit heavy (maybe it gets partially escape 
analyzed out though).  Can you instead loop over all the chars in the 
string and build up the hash code as you go along? If you see a % then 
you handle next 2 chars specially, like you do now.  Or are you trying 
to take advantage of String intrinsic support in the JIT? I guess if 
perf is a concern you can write a micro benchmark comparing the 
approaches ...


That did occur to me, but I guess we have to be consistent with the 
value that people have already been using, and that means I have
to duplicate the code in String.hashCode() (that is what the original 
implementation was calling) - I was trying to avoid that. Also,
String.hashCode() uses its internal char[] - whereas charAt() will 
involve several additional bound checks - but
using toCharArray() may be better. Let me take another look at this, and 
get back with another webrev.



Sent from my phone

On Jan 8, 2013 9:45 PM, "Vitaly Davidovich" > wrote:


Hi Kurchi,

In the hash method, I suggest you move handling of strings with %
into a separate method to keep the hash method small for common
case (no %).  Otherwise, there's a chance this method won't get
inlined anymore due to its (newly increased) size.


- Yep, will do.


Also, I realize toLower does the same thing, but why does toUpper
return an int whereas it's really a char? Might be cleaner to
declare return type as char and do the cast inside the method as
needed.

- I followed the format of toLower(). But I agree this way it will be 
cleaner.


Thanks a lot,
Kurchi




Thanks

Sent from my phone

On Jan 8, 2013 8:20 PM, "Kurchi Hazra"
mailto:kurchi.subhra.ha...@oracle.com>> wrote:

Hi,

According to RFC 3986[1], hexadecimal digits encoded by a
'%' should be case-insensitive, for example,%A2 and %a2 should be
considered equal. Although, URI.equals() does take this into
consideration, the implementation of URI.hashCode() does not and
returns different hashcodes for two URIs that are similar in
all respects except for the case of the percent-encoded
hexadecimal digits.
This fix attempts to construct a normalized string from the
string representing a component before calculating its hashCode.
I converted to upper case for the normalization(and not lower
case) as required by [1].

For testing the fix, I added an additional test scenario
to an existing test (jdk/test/java/net/URI/Test.java). While I
was there, I also made
minor changes to the test so that it does not produce rawtype
and other lint warnings.

Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7171415
Webrev: http://cr.openjdk.java.net/~khazra/7171415/webrev.00/


URI.compareTo() still suffers from the same problem - I am not
sure if it should be dealt with as a separate bug.

Thanks,
Kurchi

[1] http://tools.ietf.org/html/rfc3986#section-6.2.2.1

Re: RFR: (JDK-8005406) HTTP server implementation should use Base64 API

[Kurchi Subhra Hazra]

The change looks ok. You might want to move the comments
on this bug to 8006153 for the sake of book-keeping.

Thanks,
Kurchi

On 1/14/13 2:24 PM, Mark Sheppard wrote:

Hi,

Request for review of JDK-8005406, this time!

This is the second in a series of refactorings which migrate base64 support in 
various
packages to utilize the base64 support from java.util.Base64.

This is the modification of com.sun.net.httpserver.BasicAuthenticator.java
to use java.util.Base64.

This makes changes to the BasicAuthenticator class to use 
java.util.Base64.Decoder, and removes a
package private Base64 class.

webrev located at
http://cr.openjdk.java.net/~chegar/8005406

regards
Mark Sheppard

hg: jdk8/tl/jdk: 7171415: java.net.URI.equals/hashCode not consistent for some URIs

[kurchi . subhra . hazra]

Changeset: e8414d69692c
Author:khazra
Date:  2013-01-17 14:50 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e8414d69692c

7171415: java.net.URI.equals/hashCode not consistent for some URIs
Summary: Rewrite URI.hashCode() to consider encoded characters, also reviewed 
by vita...@gmail.com, schlo...@gmail.com
Reviewed-by: chegar

! src/share/classes/java/net/URI.java
! test/java/net/URI/Test.java

Re: RFR 8006560: java/net/ipv6tests/B6521014.java fails intermittently

[Kurchi Subhra Hazra]

Looks good - I looked into the bug details for the test too, but could 
not find any

reason why we would need server and client to bind to consecutive ports.

Thanks,
- Kurchi

On 1/18/13 6:23 AM, Chris Hegarty wrote:
This test can be seen to fail intermittently on a very busy system. 
The test tries to bind to a "hardcoded" (relative to another) port 
number. I see no reason for specify the port number in this testcase. 
The socket needs to be bound to a specific address, but we should be 
able to specify a port of 0 (ephemeral port).


Exception:
  Caused by: java.net.BindException: Address already in use
  at java.net.PlainSocketImpl.socketBind(Native Method)
  at 
java.net.AbstractPlainSocketImpl.bind(AbstractPlainSocketImpl.java:382)

  at java.net.Socket.bind(Socket.java:626)
  at B6521014.test2(B6521014.java:105)
  at B6521014.main(B6521014.java:123)

>: hg diff java/net/ipv6tests/B6521014.java
diff -r a546d8897e0d test/java/net/ipv6tests/B6521014.java
--- a/test/java/net/ipv6tests/B6521014.java Wed Jan 16 12:09:35 
2013 +
+++ b/test/java/net/ipv6tests/B6521014.java Fri Jan 18 14:18:44 
2013 +

@@ -95,14 +95,12 @@ public class B6521014 {
 Socket sock;
 ServerSocket ssock;
 int port;
-int localport;

 ssock = new ServerSocket(0);
 ssock.setSoTimeout(100);
 port = ssock.getLocalPort();
-localport = port + 1;
 sock = new Socket();
-sock.bind(new InetSocketAddress(sin, localport));
+sock.bind(new InetSocketAddress(sin, 0));
 try {
 sock.connect(new InetSocketAddress(sin, port), 100);
 } catch (SocketTimeoutException e) {

-Chris.

Re: Code review request 7183373: URLClassloader.close() does not close JAR files whose resources have been loaded via getResource()

[Kurchi Subhra Hazra]

The change looks consistent with what we had already (findResource() 
will now silently consume
an UnknownServiceException instead of a NullPointerException for 
MailToURLConnection).
Also, I noticed that  the test does not fail on Mac OS X even without 
the fix - it does fail on Windows

though.
The test will probably need some minor reformatting(some lines are 
greater than the standard

80 characters).

Thanks,
Kurchi



On 1/21/13 12:20 AM, Frank Ding wrote:

Hi Chris,
  Thanks for your review.  I did jtreg test on net module (java/net 
and sun/net) and no regression issue was found.  Could anybody else 
review the patch as well?


Best regards,
Frank

On 1/18/2013 10:55 PM, Chris Hegarty wrote:
I haven't been able to spend as much time on this as I would like, 
jdk8 M6 code freeze is approaching fast. Since this area is fraught 
with danger the safest change would be what is in the .2 version of 
the webrev [1]. I think I would be ok with this.


-Chris.

[1] http://cr.openjdk.java.net/~shirishk/7183373/webrev.2/

On 18/01/2013 06:54, Frank Ding wrote:

Hi Michael,
Could you please take a look at my comment below?

Best regards,
Frank

On 1/6/2013 4:23 PM, Frank Ding wrote:

Hi Michael,
After reading carefully discussion thread, let me elaborate my
investigation and conclusion.
The 2nd version of Shirish's patch tries to address your concern that
"Would it be possible to fix this within the context of whatever
loader is currently being invoked?". The new solution sticks to using
Loader rather than JarLoader.
The call cl.close() in the jtreg test case, according to its spec
(URLClassLoader.close) should "close any files that were opened by it
in case of jar". Its implementation code shows it closes any opened
resources through api such as getResourceAsStream invoked by client
code but doesn't take care of any resources opened by
findResource(String) or findResources(String). This implies that
findResource should return any resource found but should not leave it
in open state. The key issue for a Loader.findResource() when
searching within a jar file does not follow this rule because the code
combination "InputStream is = url.openStream(); is.close();" (in
URLClassPath.Loader.findResource()) leaves the jar file in open state.
As Shirish pointed out, if useCaches is set to true, the problem is
gone. It can be easily verified from code JarURLInputStream.close()
defined in JarURLConnection.java.
My conclusion is that Shirish's first patch is reasonable (except the
constructor change which I have not fully understood yet) because
choosing a JarLoader avoids unclosed resources after calling
URLClassLoader.getResource() and 2nd patch also makes sense as
explained above. The ramifications of these 2 patches need deliberate
considerations but we still have to fix the issue after weighing their
risks. Could you please shed your light on it?

Best regards,
Frank

On 8/25/2012 12:02 AM, Shirish Kuncolienkar wrote:

On 8/24/2012 5:39 PM, Michael McMahon wrote:

On 23/08/12 18:50, Shirish Kuncolienkar wrote:

Could I get the change reviewed please

This behavior is seen on Windows.
Logic in URLClassPath.getLoader() does not take care of an URL
which looks like "jar:file:/C:/test/xyz.jar!/". The logic ends up
choosing a FileLoader instead of a JarLoader. JarLoader has
provision for closing file handles, so choosing a JarLoader will
solve the problem. Secondly the constructor of JarLoader blindly
adds a prefix and suffix to the provided URL to make it look like a
jar URL. Changed the code here to conditionally append/prepend

The change set can be found at
http://cr.openjdk.java.net/~shirishk/7183373/webrev.0/

-Shirish


Shirish,

I have a slight concern that this would modify the Loader class to
be used in some circumstances
completely independent of the requirements of
URLClassLoader.close(). This is very sensitive code.
Would it be possible to fix this within the context of whatever
loader is currently being invoked?

- Michael


Michael,

Thanks for the review comments. The second version of the fix is
uploaded at http://cr.openjdk.java.net/~shirishk/7183373/webrev.2/
Could you please take a look at this one ?

Description of the fix:
URLClassPath.Loader.findResource() method opens a connection to the
provided URL to test whether the URL is good. Here the Jar file gets
opened but does not get closed because the created stream as
setUseCaches set to true.

Just out of curiosity I would like to know bit more on "some
circumstances completely independent of the requirements of
URLClassLoader.close()". I see that the Loader classes are private in
nature and are being used within the context of the URLClassPath.
We create an instance of JarLoader for all the jars that are on the
extension class loader path by adding "jar" , "!/" to the file url
which comes as the input. The reason behind the first fix was that if
we have a url like this why not use a JarLoader instance.

- Shirish

Re: Code review request 7183373: URLClassloader.close() does not close JAR files whose resources have been loaded via getResource()

[Kurchi Subhra Hazra]

Looks good to me.

Thanks,
Kurchi


On 1/22/13 6:50 PM, Frank Ding wrote:

Hi Chris and Kurchi,
  Thank you both for your comments.  I have reformatted the patch 
accordingly @

  http://cr.openjdk.java.net/~dingxmin/7183373/webrev.03/
  Please take a look again :-)

Best regards,
Frank

On 1/22/2013 4:42 AM, Chris Hegarty wrote:

On 01/21/2013 08:36 PM, Kurchi Subhra Hazra wrote:

The change looks consistent with what we had already (findResource()
will now silently consume
an UnknownServiceException instead of a NullPointerException for
MailToURLConnection).
Also, I noticed that  the test does not fail on Mac OS X even without
the fix - it does fail on Windows
though.


This is really just a Windows specific issue, where not closing the 
stream will prevent the file from being deleted. It is ok to have a 
general test that runs on all platforms, but well done for noticing!



The test will probably need some minor reformatting(some lines are
greater than the standard
80 characters).


Agreed, it would be nicer to reformat some lines where applicable.

Also, can you put the copyright header at the top of the file, and 
move the imports below it. And update the year to 2013, or year range 
( if applicable ).


-Chris.



Thanks,
Kurchi



On 1/21/13 12:20 AM, Frank Ding wrote:

Hi Chris,
  Thanks for your review.  I did jtreg test on net module (java/net
and sun/net) and no regression issue was found.  Could anybody else
review the patch as well?

Best regards,
Frank

On 1/18/2013 10:55 PM, Chris Hegarty wrote:

I haven't been able to spend as much time on this as I would like,
jdk8 M6 code freeze is approaching fast. Since this area is fraught
with danger the safest change would be what is in the .2 version of
the webrev [1]. I think I would be ok with this.

-Chris.

[1] http://cr.openjdk.java.net/~shirishk/7183373/webrev.2/

On 18/01/2013 06:54, Frank Ding wrote:

Hi Michael,
Could you please take a look at my comment below?

Best regards,
Frank

On 1/6/2013 4:23 PM, Frank Ding wrote:

Hi Michael,
After reading carefully discussion thread, let me elaborate my
investigation and conclusion.
The 2nd version of Shirish's patch tries to address your concern 
that

"Would it be possible to fix this within the context of whatever
loader is currently being invoked?". The new solution sticks to 
using

Loader rather than JarLoader.
The call cl.close() in the jtreg test case, according to its spec
(URLClassLoader.close) should "close any files that were opened 
by it

in case of jar". Its implementation code shows it closes any opened
resources through api such as getResourceAsStream invoked by client
code but doesn't take care of any resources opened by
findResource(String) or findResources(String). This implies that
findResource should return any resource found but should not 
leave it

in open state. The key issue for a Loader.findResource() when
searching within a jar file does not follow this rule because 
the code

combination "InputStream is = url.openStream(); is.close();" (in
URLClassPath.Loader.findResource()) leaves the jar file in open 
state.

As Shirish pointed out, if useCaches is set to true, the problem is
gone. It can be easily verified from code JarURLInputStream.close()
defined in JarURLConnection.java.
My conclusion is that Shirish's first patch is reasonable 
(except the

constructor change which I have not fully understood yet) because
choosing a JarLoader avoids unclosed resources after calling
URLClassLoader.getResource() and 2nd patch also makes sense as
explained above. The ramifications of these 2 patches need 
deliberate
considerations but we still have to fix the issue after weighing 
their

risks. Could you please shed your light on it?

Best regards,
Frank

On 8/25/2012 12:02 AM, Shirish Kuncolienkar wrote:

On 8/24/2012 5:39 PM, Michael McMahon wrote:

On 23/08/12 18:50, Shirish Kuncolienkar wrote:

Could I get the change reviewed please

This behavior is seen on Windows.
Logic in URLClassPath.getLoader() does not take care of an URL
which looks like "jar:file:/C:/test/xyz.jar!/". The logic 
ends up

choosing a FileLoader instead of a JarLoader. JarLoader has
provision for closing file handles, so choosing a JarLoader will
solve the problem. Secondly the constructor of JarLoader blindly
adds a prefix and suffix to the provided URL to make it look 
like a

jar URL. Changed the code here to conditionally append/prepend

The change set can be found at
http://cr.openjdk.java.net/~shirishk/7183373/webrev.0/

-Shirish


Shirish,

I have a slight concern that this would modify the Loader 
class to

be used in some circumstances
completely independent of the requirements of
URLClassLoader.close(). This is very sensitive code.
Would it be possible to fix this within the context of whatever
loader is currently being invoked?

- Michael


Michael,

Thanks for the review comments. The second version of the fix is
uploaded at http

hg: jdk8/tl/jdk: 7017962: Obsolete link is used in URL class level spec

[kurchi . subhra . hazra]

Changeset: 77bde15bc6a9
Author:khazra
Date:  2013-01-25 11:52 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/77bde15bc6a9

7017962: Obsolete link is used in URL class level spec
Summary: Change the link to an archived document
Reviewed-by: chegar, mduigou

! src/share/classes/java/net/URL.java

Re: Request for review: 8009650 - HttpClient available() check throws SocketException when connection has been closed

[Kurchi Subhra Hazra]

I am wondering why do you need two try-catch blocks here.

- Kurchi

On 3/7/13 8:18 AM, Rob McKenna wrote:

Hi folks,

This is a slight alteration of the fix contributed by Stuart Douglas. 
This fix deals with a SocketException caused by getSoTimeout() on a 
closed connection.


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

-Rob

Re: DefaultProxySelector socks override

[Kurchi Subhra Hazra]

On 4/2/13 6:27 PM, chris...@zoulas.com wrote:

On Apr 2,  5:26pm, kurchi.subhra.ha...@oracle.com (Kurchi Hazra) wrote:
-- Subject: Re: DefaultProxySelector socks override

| Also, I should have clarified why I am changing the test. Since now we
| are defining the socks non-proxy property -
| localhost gets added to the list of non-proxy hosts by default in our
| implementation (as is the case with other protocols
| too). So "localhost" no more acts as the socksProxyHost.

Ah, ok. Makes sense; still the change in the test does not appear to
do anything different?!? Am I missing something?


Hi Christos,

It is ok to push the test changes alongwith the code changes - we do not 
want

to break our tests as a result of new changes..
I am making an attempt at reading the actual IP address of the machine that
the test is running on, rather than the string "localhost". It is a 
workaround,
the other option is to remove the test - I do not see any other way of 
automating

what the changed test is trying to check.

- Kurchi

Re: DefaultProxySelector socks override

[Kurchi Subhra Hazra]

So the default non-proxy hosts are localhost|127.*|[::1]|0.0.0.0|[::0].
There can thus be a problem only if the local machine is being used as a 
proxy, which will be

unlikely in real environments.

We could go a step further and allow setting socksNonProxyHosts to "" 
(an empty string) to override the default. I am

not sure if it is required though.

- Kurchi

On 4/3/13 9:55 AM, Chris Hegarty wrote:

Thank you Kurchi for taking this in, and running/updating the test.

Do you think that anyone might be surprised by this change, like our 
test was? Wanting to proxy connections to the localhost seems more 
like testing environments rather than deployment.


-Chris.

On 04/03/2013 01:13 AM, Kurchi Hazra wrote:

Hi Christos/Chris,

Here is a webrev for this change:
http://cr.openjdk.java.net/~khazra/5001942/webrev.00/

Thanks,
- Kurchi



On 3/27/2013 10:49 AM, chris...@zoulas.com wrote:

On Mar 27,  5:30pm, chris.hega...@oracle.com (Chris Hegarty) wrote:
-- Subject: Re: DefaultProxySelector socks override

| On 03/27/2013 05:22 PM, chris...@zoulas.com wrote:
| > 
| > Sure, I just requested a subscription to net-dev so I might not
see the
| > first few messages. To clarify:
| >
| > 1. I will add socks.proxyHost and socks.proxyPort for 
consistency

| >with the other protocols, leaving as is socksProxyHost and
| >socksProxyPort for compatibility.
| > 2. I will add socks.nonProxyHosts and not socksNonProxyHosts.
| >
| > Is that what you had in mind?
|
| Re-checking the code I take back my previous comment. We already have
|
| socksProxyHost, socksProxyPort, socksProxyVersion
|
| so your original proposal of 'socksNonProxyHosts' is probably 
best, and

| consistent with existing properties.

I concur. Nothing for me to do :-)

Best,

christos

Re: DefaultProxySelector socks override

[Kurchi Subhra Hazra]

Sure Christos, I will include a comment in the test. I should also 
include the bug id in the jtreg tag, now

that I am changing the test.

- Kurchi

On 4/3/13 1:22 PM, chris...@zoulas.com wrote:

On Apr 3,  9:15am, kurchi.subhra.ha...@oracle.com (Kurchi Subhra Hazra) wrote:
-- Subject: Re: DefaultProxySelector socks override

| Hi Christos,
|
| It is ok to push the test changes alongwith the code changes - we do not
| want
| to break our tests as a result of new changes..
| I am making an attempt at reading the actual IP address of the machine that
| the test is running on, rather than the string "localhost". It is a
| workaround,
| the other option is to remove the test - I do not see any other way of
| automating
| what the changed test is trying to check.

I guess it is fine, but put a comment to clearly indicate why you are doing
this. But this is just my suggestion, I clearly am not the one to make a
decision :-)

christos

Re: DefaultProxySelector socks override

[Kurchi Subhra Hazra]

Alright, thanks, I'll push it today.

- Kurchi

On 4/4/13 1:33 AM, Chris Hegarty wrote:

I'm happy to go with what you have Kurchi.

-Chris.

On 04/04/2013 01:57 AM, Kurchi Subhra Hazra wrote:

So the default non-proxy hosts are localhost|127.*|[::1]|0.0.0.0|[::0].
There can thus be a problem only if the local machine is being used as a
proxy, which will be
unlikely in real environments.

We could go a step further and allow setting socksNonProxyHosts to ""
(an empty string) to override the default. I am
not sure if it is required though.

- Kurchi

On 4/3/13 9:55 AM, Chris Hegarty wrote:

Thank you Kurchi for taking this in, and running/updating the test.

Do you think that anyone might be surprised by this change, like our
test was? Wanting to proxy connections to the localhost seems more
like testing environments rather than deployment.

-Chris.

On 04/03/2013 01:13 AM, Kurchi Hazra wrote:

Hi Christos/Chris,

Here is a webrev for this change:
http://cr.openjdk.java.net/~khazra/5001942/webrev.00/

Thanks,
- Kurchi



On 3/27/2013 10:49 AM, chris...@zoulas.com wrote:

On Mar 27,  5:30pm, chris.hega...@oracle.com (Chris Hegarty) wrote:
-- Subject: Re: DefaultProxySelector socks override

| On 03/27/2013 05:22 PM, chris...@zoulas.com wrote:
| > 
| > Sure, I just requested a subscription to net-dev so I might not
see the
| > first few messages. To clarify:
| >
| > 1. I will add socks.proxyHost and socks.proxyPort for
consistency
| >with the other protocols, leaving as is socksProxyHost and
| >socksProxyPort for compatibility.
| > 2. I will add socks.nonProxyHosts and not socksNonProxyHosts.
| >
| > Is that what you had in mind?
|
| Re-checking the code I take back my previous comment. We already 
have

|
| socksProxyHost, socksProxyPort, socksProxyVersion
|
| so your original proposal of 'socksNonProxyHosts' is probably
best, and
| consistent with existing properties.

I concur. Nothing for me to do :-)

Best,

christos

hg: jdk8/tl/jdk: 5001942: Missings SOCKS support for direct connections

[kurchi . subhra . hazra]

Changeset: b702977e7212
Author:khazra
Date:  2013-04-05 12:12 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b702977e7212

5001942: Missings SOCKS support for direct connections
Summary: Add support for socksNonProxyHosts
Reviewed-by: chegar, khazra
Contributed-by: Christos Zoulas 

! src/share/classes/sun/net/spi/DefaultProxySelector.java
! test/java/net/Socks/SocksProxyVersion.java

Re: RFR-JDK8012108

[Kurchi Subhra Hazra]

Hi John,

   Minor nit, the formatting around line 101 looks off,
How about something like this:
if {
// remains same
} else {
adapterInfo = adapterInfoTemp;
}

- Kurchi

On 4/19/13 2:33 PM, John Zavgren wrote:

Greetings:

I fixed the bad realloc pattern. Please let me know what you think.
http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/

Thanks!
John Z


- Original Message -
From: chris.hega...@oracle.com
To: net-dev@openjdk.java.net, john.zavg...@oracle.com
Cc: dmitry.samers...@oracle.com
Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern
Subject: Re: RFR-JDK8012108

On 18/04/2013 22:11, Dmitry Samersoff wrote:

John,

I see bad realloc pattern here. Could you fix it as well?

Yes, please. Otherwise the changes look fine.

-Chris.


e.g.

93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len);

-Dmitry

On 2013-04-19 00:56, John Zavgren wrote:

Greetings:

I fixed a case in the windows native code where calloc() was being used
without checking it's returned value.

http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/

Thanks!
John Zavgren

Re: RFR-JDK8012108

[Kurchi Subhra Hazra]

On Apr 20, 2013, at 3:16 AM, Dmitry Samersoff  
wrote:

> John,
> 
> 102, 145 else is redundant.
> 
> 438  - we will crash here if start is null


Maybe it is good to have an additional check for start != null, but from what I 
see, start will not be null if *netaddrPP is not null.





> 
> -Dmitry
> 
> 
> On 2013-04-20 01:33, John Zavgren wrote:
>> Greetings:
>> 
>> I fixed the bad realloc pattern. Please let me know what you think.
>> http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/
>> 
>> Thanks!
>> John Z
>> 
>> 
>> - Original Message -
>> From: chris.hega...@oracle.com
>> To: net-dev@openjdk.java.net, john.zavg...@oracle.com
>> Cc: dmitry.samers...@oracle.com
>> Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern
>> Subject: Re: RFR-JDK8012108
>> 
>> On 18/04/2013 22:11, Dmitry Samersoff wrote:
>>> John,
>>> 
>>> I see bad realloc pattern here. Could you fix it as well?
>> 
>> Yes, please. Otherwise the changes look fine.
>> 
>> -Chris.
>> 
>>> 
>>> e.g.
>>> 
>>> 93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len);
>>> 
>>> -Dmitry
>>> 
>>> On 2013-04-19 00:56, John Zavgren wrote:
 Greetings:
 
 I fixed a case in the windows native code where calloc() was being used
 without checking it's returned value.
 
 http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/
 
 Thanks!
 John Zavgren
> 
> 
> -- 
> Dmitry Samersoff
> Oracle Java development team, Saint Petersburg, Russia
> * Give Rabbit time, and he'll always get the answer

Re: RFR-JDK8012108

[Kurchi Subhra Hazra]

On Apr 20, 2013, at 4:40 AM, Dmitry Samersoff  
wrote:

> Kurchi,
> 
> if  *netaddrPP == NULL at 367 and calloc fails at 391 we would jump
> to freeAllocatedMemory with start == NULL
> 


True, but then we skip to line 444 since *netaddrPP == NULL, so we don't get to 
line 438.

I am just saying it is not strictly necessary to check if start is null before 
entering the first if block. We might want to guard against how the code 
changes in future and put in the check though.


> 
> I have no ideas whether this code path is possible in reality or not,
> but it stops my eyes.
> 
> -Dmitry
> 
> 
> On 2013-04-20 14:55, Kurchi Subhra Hazra wrote:
>> 
>> 
>> On Apr 20, 2013, at 3:16 AM, Dmitry Samersoff  
>> wrote:
>> 
>>> John,
>>> 
>>> 102, 145 else is redundant.
>>> 
>>> 438  - we will crash here if start is null
>> 
>> 
>> Maybe it is good to have an additional check for start != null, but from 
>> what I see, start will not be null if *netaddrPP is not null.
>> 
>> 
>> 
>> 
>> 
>>> 
>>> -Dmitry
>>> 
>>> 
>>> On 2013-04-20 01:33, John Zavgren wrote:
>>>> Greetings:
>>>> 
>>>> I fixed the bad realloc pattern. Please let me know what you think.
>>>> http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/
>>>> 
>>>> Thanks!
>>>> John Z
>>>> 
>>>> 
>>>> - Original Message -
>>>> From: chris.hega...@oracle.com
>>>> To: net-dev@openjdk.java.net, john.zavg...@oracle.com
>>>> Cc: dmitry.samers...@oracle.com
>>>> Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern
>>>> Subject: Re: RFR-JDK8012108
>>>> 
>>>> On 18/04/2013 22:11, Dmitry Samersoff wrote:
>>>>> John,
>>>>> 
>>>>> I see bad realloc pattern here. Could you fix it as well?
>>>> 
>>>> Yes, please. Otherwise the changes look fine.
>>>> 
>>>> -Chris.
>>>> 
>>>>> 
>>>>> e.g.
>>>>> 
>>>>> 93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len);
>>>>> 
>>>>> -Dmitry
>>>>> 
>>>>> On 2013-04-19 00:56, John Zavgren wrote:
>>>>>> Greetings:
>>>>>> 
>>>>>> I fixed a case in the windows native code where calloc() was being used
>>>>>> without checking it's returned value.
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/
>>>>>> 
>>>>>> Thanks!
>>>>>> John Zavgren
>>> 
>>> 
>>> -- 
>>> Dmitry Samersoff
>>> Oracle Java development team, Saint Petersburg, Russia
>>> * Give Rabbit time, and he'll always get the answer
> 
> 
> -- 
> Dmitry Samersoff
> Oracle Java development team, Saint Petersburg, Russia
> * Give Rabbit time, and he'll always get the answer

Re: API change for 8010464: Evolve java networking same origin policy

[Kurchi Subhra Hazra]

Ok, now this is clear to me, which probably indicates that something 
like this should be

included in the documentation.

Thanks Michael,
Kurchi

On 4/30/13 3:30 AM, Michael McMahon wrote:

Hi Kurchi,

I can include such an example easily. Eg:

"GET,POST:Header1,Header2"

means one permission that permits either GET or POST with either or both
of the two headers. If you wanted to restrict one set of headers to GET
and another set to POST, then that would require two different
permissions.

- Michael

On 30/04/13 00:40, Kurchi Hazra wrote:

Hi Michael,

 From the documentation, it is not clear to me how to represent 
both request-headers and method list together in an actions string 
for two or more methods. (Say I have two methods GET and POST and I want
to specify a request-headers list for each, how do I do it?) Maybe 
another example will help.


Thanks,
Kurchi

On 4/29/2013 3:53 AM, Michael McMahon wrote:

On 28/04/13 09:01, Chris Hegarty wrote:

In the main I link the new HttpURLPermission class.

When reading the docs I found the references to "the URL" and "URL 
string" confusing ( it could be just me ). When I see capital 'URL' 
my mind instantly, and incorrectly, goes to java.net.URL. In all 
cases you mean the URL string given when constructing the 
HttpURLPermission, right?




Yes, that is what is meant. The class does not use j.n.URL at all, 
as that would bring us back
to the old (undesirable) behavior with DNS lookups required for 
basic operations like equals() and hashCode()



Another example is the equals method
  "Returns true if, this.getActions().equals(p.getActions()) and p's
   URL equals this's URL. Returns false otherwise."

this is referring so a simple string comparison of the given URL 
string, right? This should be case insensitive too. Does it take 
into account default protocol ports, e.g. http://foo.com/ equal 
http://foo.com:80/




The implementation uses a java.net.URI internally. So URI takes care 
of that.


implies() makes reference to the URL scheme, and other specific 
parts of the URL. Also, the constructors throw IAE  'if url is not 
a valid URL', but what does this mean. Should we just bite the 
bullet and just say that URI is used to parse the given string into 
its specific parts? Otherwise, how can this be validated.




I originally didn't want to mention URI in the apidoc due to 
potential confusion surrounding the use of URL in the permission
class name. But, maybe it would be clearer to be explicit about it, 
particularly for the equals() behavior.

Otherwise we have to specify all of it in this class.

As for the additions to HttpURLConnection, what are the 
implications on proxies? Permissions, etc...




There's no change in behavior with respect to proxies. Permission is 
given to connect to proxies implicitly
except in cases where the caller specifies the proxy through the 
URL.openConnection(Proxy) api.
There are other unusual cases like the Http "Use Proxy" response. 
Explicit permission is required

for that case also.

Thanks!

Michael


-Chris.

On 04/26/2013 03:36 PM, Michael McMahon wrote:

Hi,

The is the suggested API for one of the two new JEPs recently 
submitted.


This is for JEP 184: HTTP URL Permissions

The idea here is to define a higher level http permission class
which "knows about" URLs, HTTP request methods and headers.
So, it is no longer necessary to grant blanket permission for any 
kind
of TCP connection to a host/port. Instead a HttpURLPermission 
restricts
access to only the Http protocol itself. Restrictions can also be 
imposed

based on URL paths, specific request methods and request headers.

The API change can be seen at the URL below:

http://cr.openjdk.java.net/~michaelm/8010464/api/

In addition to defining a new permission class, HttpURLConnection
is modified to make use of it and the documentation change 
describing this

can be seen at the link below:

http://cr.openjdk.java.net/~michaelm/8010464/api/blender.html

All comments welcome.

Thanks

Michael.

Code Review Request: 8013140: Heap corruption with NetworkInterface.getByInetAddress() and long i/f name

[Kurchi Subhra Hazra]

Hi,

   NetworkInterface.getByInetAddress() was crashing on solaris when the system 
had a network
interface name longer than 15 characters, due to two instances in the native
code for NetworkInterface where we were copying a char array of size 32 
(LIFNAMSIZ)
into a char array of size 16 (IFNAMSIZ), resulting in a buffer overflow with 
long names.
The fix is to make sure that the space allocated for the interface name is 
consistent (16/32
bytes depending on the system), and to prevent overflows by using strncpy 
instead of strcpy.

Bug: http://bugs.sun.com/view_bug.do?bug_id=8013140
Webrev: http://cr.openjdk.java.net/~khazra/8013140/webrev.00/


Thanks,
- Kurchi

hg: jdk8/tl/jdk: 8013140: Heap corruption with NetworkInterface.getByInetAddress() and long i/f name

[kurchi . subhra . hazra]

Changeset: 3062bf908281
Author:khazra
Date:  2013-05-02 14:26 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3062bf908281

8013140: Heap corruption with NetworkInterface.getByInetAddress() and long i/f 
name
Summary: Remove buffer overruns in native code
Reviewed-by: alanb, chegar

! src/solaris/native/java/net/NetworkInterface.c

Re: Code Review Request: 8014254: Selector in HttpServer introduces a 1000 ms delay when using KeepAlive

[Kurchi Subhra Hazra]

There's something fishy here. So are there are non I/O events queued 
up that will not be serviced more quickly? Shouldn't adding events 
cause the Selector to wakeup so that it doesn't matter what the 
timeout value is.


- This doesn't quite work if we have a single thread doing everything. 
(which is why this bug is a non-issue when

the user has set a ThreadPool of size > 1 to be used by the server).

- Kurchi

hg: jdk8/tl/jdk: 8014254: Selector in HttpServer introduces a 1000 ms delay when using KeepAlive

[kurchi . subhra . hazra]

Changeset: 86c1e8c799f5
Author:khazra
Date:  2013-05-13 13:48 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/86c1e8c799f5

8014254: Selector in HttpServer introduces a 1000 ms delay when using KeepAlive
Summary: Rearrange event-handling code to remove bottle-neck. Also reviewed by 
mh...@mhcomputing.net.
Reviewed-by: chegar, alanb

! src/share/classes/sun/net/httpserver/ServerImpl.java

hg: jdk8/tl/jdk: 6328537: Improve javadocs for Socket class by adding references to SocketOptions

[kurchi . subhra . hazra]

Changeset: 790d292ee761
Author:khazra
Date:  2013-05-14 12:01 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/790d292ee761

6328537: Improve javadocs for Socket class by adding references to SocketOptions
Summary: Insert references to SocketOptions.java where applicable
Reviewed-by: alanb, chegar

! src/share/classes/java/net/ServerSocket.java
! src/share/classes/java/net/Socket.java

hg: jdk8/tl/jdk: 7150552: network test hangs [macosx]

[kurchi . subhra . hazra]

Changeset: a8be9405bb4b
Author:khazra
Date:  2013-05-16 10:58 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a8be9405bb4b

7150552: network test hangs [macosx]
Summary: Remove usage of test/sun/net/www/httptest
Reviewed-by: chegar

! test/ProblemList.txt
! test/java/net/CookieHandler/CookieManagerTest.java
! test/sun/net/www/protocol/http/B6299712.java

hg: jdk8/tl/jdk: 2 new changesets

[kurchi . subhra . hazra]

Changeset: 11cdcf87ad5d
Author:jzavgren
Date:  2013-05-31 15:23 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/11cdcf87ad5d

8008972: Memory leak: Java_java_net_TwoStacksPlainDatagramSocketImpl_receive0 
[parfait]
Summary: Modified the code so that "jumbo frames" are truncated before buffer 
allocation is considered. This makes the buffer length a reliable indication 
that a buffer has been allocated, and it can then be used during clean up.
Reviewed-by: chegar, khazra, alanb
Contributed-by: john.zavg...@oracle.com

! src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
! src/windows/native/java/net/TwoStacksPlainDatagramSocketImpl.c

Changeset: f6e6c27c19f3
Author:jzavgren
Date:  2013-05-31 15:18 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f6e6c27c19f3

7188517: Check on '$' character is missing in the HttpCookie class constructor
Summary: Modified the constructor code so that the cookie names are examined 
for leading dollar signs and if they do, an illegal argument exception is 
thrown.
Reviewed-by: chegar, khazra, michaelm
Contributed-by: john.zavg...@oracle.com

! src/share/classes/java/net/HttpCookie.java
! test/java/net/CookieHandler/TestHttpCookie.java

hg: jdk8/tl/jdk: 7051862: CookiePolicy spec conflicts with CookiePolicy.ACCEPT_ORIGINAL_SERVER

[kurchi . subhra . hazra]

Changeset: 8b65dfe8f509
Author:khazra
Date:  2013-06-07 10:59 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8b65dfe8f509

7051862: CookiePolicy spec conflicts with CookiePolicy.ACCEPT_ORIGINAL_SERVER
Summary: Return false for null arguments in 
ACCEPT_ORIGINAL_SERVER#shouldAccept()
Reviewed-by: chegar

! src/share/classes/java/net/CookiePolicy.java
! test/java/net/CookieHandler/CookieManagerTest.java

hg: jdk8/tl/jdk: 8015421: NegativeArraySizeException occurs in ChunkedOutputStream() with Integer.MAX_VALUE

[kurchi . subhra . hazra]

Changeset: ff83bd43e36a
Author:khazra
Date:  2013-06-13 11:23 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ff83bd43e36a

8015421: NegativeArraySizeException occurs in ChunkedOutputStream() with 
Integer.MAX_VALUE
Summary: Ensure integer overflow does not occur
Reviewed-by: chegar

! src/share/classes/sun/net/www/http/ChunkedOutputStream.java

hg: jdk8/tl/jdk: 7169142: CookieHandler does not work with localhost

[kurchi . subhra . hazra]

Changeset: 42f9ad39bf42
Author:khazra
Date:  2013-06-13 17:48 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/42f9ad39bf42

7169142: CookieHandler does not work with localhost
Summary: Add .local to derived effective hostnames without dot
Reviewed-by: chegar

! src/share/classes/java/net/CookieManager.java
+ test/java/net/CookieHandler/LocalHostCookie.java

hg: jdk8/tl/jdk: 8016576: Overrides warnings in jdi and jconsole

[kurchi . subhra . hazra]

Changeset: de6b93fd6d23
Author:khazra
Date:  2013-06-19 14:02 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/de6b93fd6d23

8016576: Overrides warnings in jdi and jconsole
Summary: Implement hashCode() in classes emitting warnings
Reviewed-by: alanb, chegar

! src/share/classes/com/sun/tools/jdi/SDE.java
! src/share/classes/sun/tools/jconsole/inspector/XObject.java

hg: jdk8/tl/jdk: 8016698: Cleanup overrides warning in sun/tools/ClassDeclaration.java

[kurchi . subhra . hazra]

Changeset: e1b18a666f76
Author:khazra
Date:  2013-06-19 14:13 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e1b18a666f76

8016698: Cleanup overrides warning in sun/tools/ClassDeclaration.java
Summary: Override Object.hashCode()
Reviewed-by: alanb, chegar

! src/share/classes/sun/tools/java/ClassDeclaration.java

hg: jdk8/tl/jdk: 7025238: HttpURLConnection does not handle URLs with an empty path component.

[kurchi . subhra . hazra]

Changeset: 2b156531b7eb
Author:arieber
Date:  2013-06-19 17:41 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2b156531b7eb

7025238: HttpURLConnection does not handle URLs with an empty path component.
Summary: Prepend a '/' to file when path is empty
Reviewed-by: chegar, khazra

! src/share/classes/sun/net/www/http/HttpClient.java
+ test/sun/net/www/http/HttpClient/B7025238.java

Re: Code Review Request: 8017779: java/net/Authenticator/B4769350.java fails

[Kurchi Subhra Hazra]

Hi,

Did anyone have a chance to look at this?

Thanks,
Kurchi


On Thu, Jul 18, 2013 at 4:26 PM, Kurchi Hazra <
kurchi.subhra.ha...@oracle.com> wrote:

> Hi Michael,
>
>I added some comments as to what is the purpose of the latches and
> barriers. Those comments alongwith the
> comments describing the purpose of the handlers should make the
> synchronization logic more clear. Let me know what
> you think: 
> http://cr.openjdk.java.net/~**khazra/8017779/webrev.01/
>
> Thanks,
> Kurchi
>
> On 7/17/2013 2:07 PM, Kurchi Hazra wrote:
>
>>
>> On 7/17/2013 12:27 AM, Michael McMahon wrote:
>>
>>> On 16/07/13 20:11, Kurchi Hazra wrote:
>>>
 Hi,
  We have observed that test/java/net/Authenticator/**B4769350.java
 fails intermittently. Although not reproducible always,
 the bug could be in the test/sun/net/www/httptest library that this
 test uses. I have rewritten the test to use
 com.sun.net.httpserver instead since we anyway want to move away from
 using the httptest library.

 I have used CyclicBarriers to mimic TestHttpServer.rendezvous() and
 CountDownLatches to
 mimic TestHttpServer.**waitForCondition() and hopefully preserved the
 rest of the logic in the test.
 I have not seen the test failing after these changes.

 Bug: 
 http://bugs.sun.com/view_bug.**do?bug_id=8017779
 Webrev: 
 http://cr.openjdk.java.net/~**khazra/8017779/webrev.00/

 Thanks,
 Kurchi


>>> Kurchi,
>>>
>>> Since this is a fairly complicated test, and it's great to see it being
>>> rewritten,
>>> is there any possibility of adding some commentary that explains the
>>> purpose
>>> of the synchronization code. For instance, I can't see the purpose of
>>> the call
>>> on line 163 as it just blocks a thread that has already completed its
>>> work
>>>
>>> Michael
>>>
>>
>> Hi Michael,
>>
>>   I have just preserved whatever logic the test originally had. The
>> specific instance you point out waits
>> for the T1b() handle to be executed for case 0 before moving forward. But
>> I guess it is hard to understand in a
>> glance and I'll add some more comments and get back with a new webrev.
>>
>> Thanks,
>> Kurchi
>>
>
> --
> -Kurchi
>
>

Re: RFR: 8023326 [TESTBUG] java/net/CookieHandler/LocalHostCookie.java misplaced try/finally

[Kurchi Subhra Hazra]

I guess we could have changed class Server to implement AutoCloseable too,
but this looks fine to me.

Thanks,
- Kurchi


On Wed, Aug 21, 2013 at 6:37 AM, Mark Sheppard wrote:

> Hi
> please oblige and review the fix below to address the issue in JDK-8023326
> which makes the test more robust to certain exceptions thrown.
>
> http://cr.openjdk.java.net/~**msheppar/8023326/webrev/
>
> regards
> Mark
>
> --- old/test/java/net/**CookieHandler/LocalHostCookie.**javaWed
> Aug 21 12:23:21 2013
> +++ new/test/java/net/**CookieHandler/LocalHostCookie.**javaWed
> Aug 21 12:23:21 2013
> @@ -72,7 +72,9 @@
>  }
>  }
>  } finally {
> -s.stopServer();
> +if (s != null) {
> +s.stopServer();
> +}
>  }
>  }
>  @@ -96,7 +98,9 @@
>  }
>   public void stopServer() {
> -server.stop(0);
> +if (server != null) {
> +server.stop(0);
> +}
>  }
>  }
>
>
>
>