Code review request: 8009970: Several LoginModule classes need extra permission to load AuthResources
http://cr.openjdk.java.net/~weijun/8009970/webrev.00 I was checking for krb5 permissions and noticed this. There is really no need for a permission to access AuthResources strings in these login modules. Also change to private, building JDK shows no other class uses the fields. Noreg-trivial. Thanks Max
Code review request: 8009977: A test library to launch multiple Java processes (using krb5 as an example)
http://cr.openjdk.java.net/~weijun/8009977/webrev.00 /** * This is a test library that makes writing a Java test that spawns multiple * Java processes easily. * * Usage: * *Proc.create(Clazz) // The class to launch *.args(x)// with args *.env(env, value) // and an environment variable *.prop(key,value) // and a system property *.perm(perm) // with granted permissions *.start(); // and start * * * The controller can call inheritIO to share its I/O to the process. * Otherwise, it can send data into a proc's stdin with write/println, and * read its stdout with readLine, and stderr is redirected to a file. Thanks Max
Re: Code review request: 8009977: A test library to launch multiple Java processes (using krb5 as an example)
Max, Does this overlap with the testlibrary for supporting muli-process tests that Katja Kantserova added recently? -Alan. On 13/03/2013 11:18, Weijun Wang wrote: http://cr.openjdk.java.net/~weijun/8009977/webrev.00 /** * This is a test library that makes writing a Java test that spawns multiple * Java processes easily. * * Usage: * *Proc.create(Clazz) // The class to launch *.args(x)// with args *.env(env, value) // and an environment variable *.prop(key,value) // and a system property *.perm(perm) // with granted permissions *.start(); // and start * * * The controller can call inheritIO to share its I/O to the process. * Otherwise, it can send data into a proc's stdin with write/println, and * read its stdout with readLine, and stderr is redirected to a file. Thanks Max
Re: RFR 8007637
Looks good to me. I can push this for you later today. --Sean On 03/12/2013 04:12 PM, John Zavgren wrote: Greetings: I posted a webrev image of a (very minor) bug fix that eliminates a reference to freed memory. http://cr.openjdk.java.net/~jzavgren/8007637/webrev.01/ Thanks! John
Re: RFR 8007637
On 13/03/2013 12:15, Sean Mullan wrote: Looks good to me. I can push this for you later today. Looks fine to me too. -Chris. --Sean On 03/12/2013 04:12 PM, John Zavgren wrote: Greetings: I posted a webrev image of a (very minor) bug fix that eliminates a reference to freed memory. http://cr.openjdk.java.net/~jzavgren/8007637/webrev.01/ Thanks! John
hg: jdk8/tl/jdk: 8009751: (se) Selector spin when select, close and interestOps(0) invoked at same time (lnx)
Changeset: e33cbbe21419 Author:alanb Date: 2013-03-13 17:58 + URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e33cbbe21419 8009751: (se) Selector spin when select, close and interestOps(0) invoked at same time (lnx) Reviewed-by: zhouyx, chegar, robm ! src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java ! src/solaris/classes/sun/nio/ch/EPollSelectorImpl.java
hg: jdk8/tl/jdk: 7190897: (fs) Files.isWritable method returns false when the path is writable (win)
Changeset: e497a050e059 Author:uta Date: 2013-03-13 13:22 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e497a050e059 7190897: (fs) Files.isWritable method returns false when the path is writable (win) Summary: The [GetEffectiveRightsFromAcl] based implementation was changed to the [AccessCheck] based. Reviewed-by: alanb ! src/windows/classes/sun/nio/fs/WindowsConstants.java ! src/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java ! src/windows/classes/sun/nio/fs/WindowsNativeDispatcher.java ! src/windows/classes/sun/nio/fs/WindowsSecurity.java ! src/windows/native/sun/nio/fs/WindowsNativeDispatcher.c
hg: jdk8/tl/jdk: 8002070: Remove the stack search for a resource bundle for Logger to use
Changeset: 94335b6ffb32 Author:jgish Date: 2013-03-13 11:24 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/94335b6ffb32 8002070: Remove the stack search for a resource bundle for Logger to use Summary: The fragile, vulnerable, stack crawling has been eliminated from findResourceBundle(String) Reviewed-by: mchung, alanb ! src/share/classes/java/util/logging/Logger.java ! test/java/util/logging/LoggerResourceBundleRace.java
hg: jdk8/tl/langtools: 8006547: Repeating annotations: No Target on container annotation with all targets on base annotation gives compiler error
Changeset: eb0198033c5c Author:jfranck Date: 2013-03-13 22:03 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/eb0198033c5c 8006547: Repeating annotations: No Target on container annotation with all targets on base annotation gives compiler error Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/comp/Check.java + test/tools/javac/annotations/repeatingAnnotations/DefaultTarget.java + test/tools/javac/annotations/repeatingAnnotations/DefaultTargetTypeParameter.java + test/tools/javac/annotations/repeatingAnnotations/DefaultTargetTypeParameter.out + test/tools/javac/annotations/repeatingAnnotations/DefaultTargetTypeUse.java + test/tools/javac/annotations/repeatingAnnotations/DefaultTargetTypeUse.out + test/tools/javac/annotations/repeatingAnnotations/NoTargetOnContainer.java + test/tools/javac/annotations/repeatingAnnotations/NoTargetOnContainer2.java
hg: jdk8/tl/jdk: 8001334: Remove use of JVM_* functions from java.io code
Changeset: ef0c60b93a17 Author:dxu Date: 2013-03-13 14:50 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ef0c60b93a17 8001334: Remove use of JVM_* functions from java.io code Summary: Replace JVM_* functions with direct system calls in java io area Reviewed-by: alanb, uta, martin ! make/java/nio/Makefile ! makefiles/CompileNativeLibraries.gmk ! src/share/native/java/io/ObjectOutputStream.c ! src/share/native/java/io/io_util.c ! src/share/native/java/io/io_util.h ! src/solaris/native/common/jdk_util_md.h ! src/solaris/native/java/io/FileDescriptor_md.c ! src/solaris/native/java/io/UnixFileSystem_md.c ! src/solaris/native/java/io/io_util_md.c ! src/solaris/native/java/io/io_util_md.h ! src/windows/native/common/jdk_util_md.h ! src/windows/native/java/io/io_util_md.c ! src/windows/native/java/io/io_util_md.h
hg: jdk8/tl/langtools: 8009684: Default top left frame should be All Packages in the generated javadoc documentation
Changeset: e0ef84e33167 Author:bpatel Date: 2013-03-13 14:47 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/e0ef84e33167 8009684: Default top left frame should be All Packages in the generated javadoc documentation Reviewed-by: jjg ! src/share/classes/com/sun/tools/doclets/formats/html/FrameOutputWriter.java ! src/share/classes/com/sun/tools/doclets/formats/html/PackageIndexFrameWriter.java ! src/share/classes/com/sun/tools/doclets/formats/html/ProfileIndexFrameWriter.java ! src/share/classes/com/sun/tools/doclets/formats/html/ProfilePackageIndexFrameWriter.java ! test/com/sun/javadoc/testProfiles/TestProfiles.java
hg: jdk8/tl/jdk: 8009650: HttpClient available() check throws SocketException when connection has been closed
Changeset: f5c85c0a9af0 Author:robm Date: 2013-03-14 00:21 + URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f5c85c0a9af0 8009650: HttpClient available() check throws SocketException when connection has been closed Reviewed-by: chegar, khazra, dsamersoff Contributed-by: sdoug...@redhat.com ! src/share/classes/sun/net/www/http/HttpClient.java + test/sun/net/www/http/HttpClient/IsAvailable.java
Re: RFR: JDK-8007607
Looks fine, just a very minor nit.
GSSLibStub.c
- line 544: although the return value isn't really used, maybe you
should return jlong_zero instead of -1 for consistency sake.
Thanks,
Valerie
On 03/12/13 08:34, John Zavgren wrote:
Greetings:
I posted a new webrev image in response to the most recent round of
comments:
http://cr.openjdk.java.net/~jzavgren/8007607/webrev.07/
http://cr.openjdk.java.net/%7Ejzavgren/8007607/webrev.07/
Thanks!
John
On 02/19/2013 12:16 PM, Chris Hegarty wrote:
Hi John,
Functionally this looks fine. Most my comments are nit picking, and
style.
src/share/native/sun/security/jgss/wrapper/GSSLibStub.c
My fault, I think I suggested you return NULL, but since these
methods return jlong they cannot (without generating warnings).
It would be better to:
332 return NULL;
---
332 return jlong_zero;
1167 return NULL; [same comment, return jlong_zero]
The indentation looks a little too much in a few places, e.g.
331 if ((*env)-ExceptionCheck(env)) {
332 __return NULL;
333 }
src/share/native/sun/security/jgss/wrapper/NativeUtil.c
620 cOid = malloc(sizeof(struct gss_OID_desc_struct));
621 if_(cOid == NULL) { [add a space after if]
622 throwOutOfMemoryError(env,NULL); [I would suggest 2
spaces]
623 return GSS_C_NO_OID;
624 }
625 cOid-length = (*env)-GetArrayLength(env, jbytes) - 2;
626 cOid-elements = malloc(cOid-length);
627 if(cOid-elements == NULL) {[ same as above ]
628 throwOutOfMemoryError(env,NULL);
629 free(cOid);
630 return GSS_C_NO_OID;
631 }
src/share/native/sun/security/smartcardio/pcsc.c
src/solaris/native/sun/security/smartcardio/pcsc_md.c
It is kinda weird to have #ifdef WIN32 for these. It really seems
that these functions should be moved up to the shared pcsc.c
and externed from platform specific code, or an addition of
pcsc.h that declares the definitions.
src/solaris/native/com/sun/security/auth/module/Solaris.c
The comment is strange
34 /*
35 * ** Throws a Java Exception by name
36 * **/
src/solaris/native/com/sun/security/auth/module/Unix.c
Strange comment ( as above ). Also, why is there a need to for
#ifndef __solaris__ ??
-Chris.
On 02/18/2013 04:09 PM, John Zavgren wrote:
Greetings:
I posted a new webrev image.
http://cr.openjdk.java.net/~jzavgren/8007607/webrev.04/
http://cr.openjdk.java.net/%7Ejzavgren/8007607/webrev.04/
The code now throws an OOM exception when *alloc() fails, and the
callers of procedures that call procedures that throw it, check for it.
John
On 02/12/2013 11:03 AM, Dmitry Samersoff wrote:
John,
Changing everything for throw OOM is the right goal but it's a huge
work
to do - it's meaningless to throw OOM if all callers doesn't check for
exception.
I'm in doubt it could be done all at once and probably this problem
should go to the huge TODO pile.
So I'm speaking about *one particular case*, where returned value
could
lead to misinterpretation of security context and lead to security
vulnerability or subtle bug.
We have to throw OOM there and change all callers as well for this
case.
-Dmitry
On 2013-02-12 19:51, John Zavgren wrote:
On 02/08/2013 01:34 PM, Dmitry Samersoff wrote:
John,
Ideas?
It's a JNI so just throw OOM.
-Dmitry
On 2013-02-08 21:38, John Zavgren wrote:
Although I agree that the name: GSS_C_NO_CHANNEL_BINDINGS is
misleading,
I can't identify anything else that seems more appropriate.
The header file:
/jdk8-tl/jdk/src/share/native/sun/security/jgss/wrapper/gssapi.h
defines
GSS_C_NO_CHANNEL_BINDINGS as follows:
#define GSS_C_NO_CHANNEL_BINDINGS ((gss_channel_bindings_t) 0)
The symbol matches the prototype of the function:
*/*
* Utility routine which creates a gss_channel_bindings_t
structure
* using the specified org.ietf.jgss.ChannelBinding object.
*/
gss_channel_bindings_t getGSSCB(JNIEnv *env, jobject jcb) {
gss_channel_bindings_t cb;
jobject jinetAddr;
jbyteArray value;
if (jcb == NULL) {
return GSS_C_NO_CHANNEL_BINDINGS;
}
cb = malloc(sizeof(struct gss_channel_bindings_struct));
if(cb == NULL)
return GSS_C_NO_CHANNEL_BINDINGS;*
There doesn't appear to be anything in our set of options that
is more
suggestive of a memory allocation failure and the symbol:
GSS_C_NO_CHANNEL_BINDINGS seems to be logically correct.
Ideas?
On 02/06/2013 04:57 AM, Dmitry Samersoff wrote:
John,
Not sure GSS_C_NO_CHANNEL_BINDINGS; is correct return value for
this
case.
I'm second to Valerie - it's better to throw OOM
-Dmitry
On 2013-02-06 03:44, John Zavgren wrote:
Greetings:
I modified the native code to eliminate potential memory loss and
crashes by checking the return values of malloc() and
realloc() calls.
The webrev image of these changes is visible at:
Re: Allow configure to detect if EC implementation is present
CC'ing security-dev. Vinnie, As owner of ECC, you should probably look at this. Brad On 3/13/2013 7:02 PM, David Holmes wrote: On 14/03/2013 6:09 AM, Omair Majid wrote: Hi, jdk/makefiles/CompileNativeLibraries.gmk has a little note: TODO Set DISABLE_INTREE_EC in configure if src/share/native/sun/security/ec/impl is not present The webrev at http://cr.openjdk.java.net/~omajid/webrevs/intree-ec/00/ implements this. Does this look okay for jdk8/build ? Can I get a bug id? Bug ID: 8010030 I think it is more consistent to set the variable to yes/no and change: ifndef DISABLE_INTREE_EC to ifeq ($DISABLE_INTREE_EC), yes) Thanks, David Thanks, Omair
Re: Allow configure to detect if EC implementation is present
Note that this isn't changing any functionality simply exposing an existing make variable at configure time. David On 14/03/2013 2:38 PM, Brad Wetmore wrote: CC'ing security-dev. Vinnie, As owner of ECC, you should probably look at this. Brad On 3/13/2013 7:02 PM, David Holmes wrote: On 14/03/2013 6:09 AM, Omair Majid wrote: Hi, jdk/makefiles/CompileNativeLibraries.gmk has a little note: TODO Set DISABLE_INTREE_EC in configure if src/share/native/sun/security/ec/impl is not present The webrev at http://cr.openjdk.java.net/~omajid/webrevs/intree-ec/00/ implements this. Does this look okay for jdk8/build ? Can I get a bug id? Bug ID: 8010030 I think it is more consistent to set the variable to yes/no and change: ifndef DISABLE_INTREE_EC to ifeq ($DISABLE_INTREE_EC), yes) Thanks, David Thanks, Omair
hg: jdk8/tl: 8009428: Revert changes to $ substitution performed as part of nashorn integration
Changeset: 19a59a13b3ef Author:dholmes Date: 2013-03-14 01:41 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/rev/19a59a13b3ef 8009428: Revert changes to $ substitution performed as part of nashorn integration Reviewed-by: alanb, erikj ! common/makefiles/MakeBase.gmk
hg: jdk8/tl/jdk: 8009429: Miscellaneous profiles cleanup; ...
Changeset: 41289b4a1819 Author:dholmes Date: 2013-03-14 01:47 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/41289b4a1819 8009429: Miscellaneous profiles cleanup 8009428: Revert changes to $ substitution performed as part of nashorn integration Reviewed-by: alanb, erikj ! makefiles/CreateJars.gmk ! makefiles/ProfileNames.gmk ! makefiles/Profiles.gmk ! makefiles/profile-includes.txt ! makefiles/profile-rtjar-includes.txt
hg: jdk8/tl/langtools: 8009429: Miscellaneous profiles cleanup
Changeset: 82dc1e827c2a Author:dholmes Date: 2013-03-14 01:45 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/82dc1e827c2a 8009429: Miscellaneous profiles cleanup Reviewed-by: jjg, alanb ! src/share/classes/com/sun/tools/javac/sym/Profiles.java
tl forest update
Sorry for the wide distribution but you all see the push messages anyway. I've just pushed a coordinated set of changes to the top-level, langtools and jdk repos in the tl forest. If you use tl you will need to ensure that you update all of these repos to ensure they are in sync. Thanks, David
