From 1b92857246b26e94fe547002ffc7c2e9ed3d233e Mon Sep 17 00:00:00 2001 From: Sean Mullan Date: Wed, 18 Nov 2009 12:34:26 -0500 Subject: [PATCH 1/5] 6894461: OCSP Checker should not wrap all Exception as "Unable to send OCSP request."(introduced by #6885667) Reviewed-by: vinnie, xuelei --- .../sun/security/provider/certpath/OCSPChecker.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/jdk/src/share/classes/sun/security/provider/certpath/OCSPChecker.java b/jdk/src/share/classes/sun/security/provider/certpath/OCSPChecker.java index 499a5912aca..3d17803f765 100644 --- a/jdk/src/share/classes/sun/security/provider/certpath/OCSPChecker.java +++ b/jdk/src/share/classes/sun/security/provider/certpath/OCSPChecker.java @@ -335,10 +335,13 @@ class OCSPChecker extends PKIXCertPathChecker { response = OCSP.check(Collections.singletonList(certId), uri, responderCert, pkixParams.getDate()); } catch (Exception e) { - // Wrap all exceptions in CertPathValidatorException so that - // we can fallback to CRLs, if enabled. - throw new CertPathValidatorException - ("Unable to send OCSP request", e); + if (e instanceof CertPathValidatorException) { + throw (CertPathValidatorException) e; + } else { + // Wrap exceptions in CertPathValidatorException so that + // we can fallback to CRLs, if enabled. + throw new CertPathValidatorException(e); + } } RevocationStatus rs = (RevocationStatus) response.getSingleResponse(certId); From 5ef31ba1520a78cfe849efd0cc602a0ec38a3c2d Mon Sep 17 00:00:00 2001 From: Kelly O'Hair Date: Wed, 18 Nov 2009 11:10:18 -0800 Subject: [PATCH 2/5] 6902323: Fix testcase sun/tools/native2ascii/NativeErrors.java 6902325: Fix testcase sun/tools/jhat/HatHeapDump1Test.java Reviewed-by: tbell, dcubed --- jdk/test/ProblemList.txt | 6 ------ jdk/test/sun/tools/jhat/HatRun.java | 4 +++- .../sun/tools/native2ascii/NativeErrors.java | 20 +++++++++++++++++-- jdk/test/sun/tools/native2ascii/test2 | 1 - 4 files changed, 21 insertions(+), 10 deletions(-) delete mode 100644 jdk/test/sun/tools/native2ascii/test2 diff --git a/jdk/test/ProblemList.txt b/jdk/test/ProblemList.txt index 9a366c1ba8f..4bced4d851b 100644 --- a/jdk/test/ProblemList.txt +++ b/jdk/test/ProblemList.txt @@ -1117,9 +1117,6 @@ sun/jvmstat/monitor/MonitoredVm/CR6672135.java generic-all # Unexpected Monitor Exception, solaris sparc -client sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh generic-all -# Probably should be samevm, but seem to cause errors even in othervm at times -sun/tools/jhat/HatHeapDump1Test.java generic-all - # Problems on windows, jmap.exe hangs? (these run jmap) sun/tools/jmap/Basic.sh windows-all @@ -1129,9 +1126,6 @@ sun/tools/jstatd/jstatdDefaults.sh solaris-all # Solaris sparcv9, jps output does not match, x64 different sun/tools/jstatd/jstatdExternalRegistry.sh solaris-all -# Probably should be samevm, but seem to cause errors even in othervm at times -sun/tools/native2ascii/NativeErrors.java generic-all - # Solaris 10 sparc 32bit -client, java.lang.AssertionError: Some tests failed tools/jar/JarEntryTime.java generic-all diff --git a/jdk/test/sun/tools/jhat/HatRun.java b/jdk/test/sun/tools/jhat/HatRun.java index cac66e57d8c..cb1e41b248e 100644 --- a/jdk/test/sun/tools/jhat/HatRun.java +++ b/jdk/test/sun/tools/jhat/HatRun.java @@ -186,11 +186,13 @@ public class HatRun { */ int nvm_options = 0; if ( vm_options != null ) nvm_options = vm_options.length; - String cmd[] = new String[1 + (d64?1:0) + 5 + nvm_options]; + String cmd[] = new String[1 + (d64?1:0) + 7 + nvm_options]; int i,j; i = 0; cmd[i++] = java; + cmd[i++] = "-cp"; + cmd[i++] = cdir; cmd[i++] = "-Dtest.classes=" + cdir; if ( d64 ) { cmd[i++] = "-d64"; diff --git a/jdk/test/sun/tools/native2ascii/NativeErrors.java b/jdk/test/sun/tools/native2ascii/NativeErrors.java index 460a9f158cf..c0fbad160e0 100644 --- a/jdk/test/sun/tools/native2ascii/NativeErrors.java +++ b/jdk/test/sun/tools/native2ascii/NativeErrors.java @@ -59,15 +59,28 @@ public class NativeErrors { in = new BufferedReader(new InputStreamReader(p.getInputStream())); checkResult(in, "err.bad.arg"); - command = getComString("test123"); + File f0 = new File(System.getProperty("test.src", "."), "test123"); + String path0 = f0.getPath(); + if ( f0.exists() ) { + throw new Error("Input file should not exist: " + path0); + } + + command = getComString(path0); p = Runtime.getRuntime().exec(command); in = new BufferedReader(new InputStreamReader(p.getInputStream())); checkResult(in, "err.cannot.read"); File f1 = new File(System.getProperty("test.src", "."), "test1"); - File f2 = new File(System.getProperty("test.src", "."), "test2"); + File f2 = File.createTempFile("test2", ".tmp"); String path1 = f1.getPath(); String path2 = f2.getPath(); + if ( !f1.exists() ) { + throw new Error("Missing input file: " + path1); + } + if ( !f2.setWritable(false) ) { + throw new Error("Output file cannot be made read only: " + path2); + } + f2.deleteOnExit(); command = getComString(path1, path2); p = Runtime.getRuntime().exec(command); @@ -80,7 +93,9 @@ public class NativeErrors { throws Exception { String errorReceived; errorReceived = in.readLine(); + assert errorReceived != null : "First readline cannot be null"; errorExpected = rsrc.getString(errorExpected); + assert errorExpected != null : "Expected message cannot be null"; StringBuffer error = new StringBuffer(errorExpected); int start = errorExpected.indexOf("{0}"); if (start >= 0) { @@ -128,6 +143,7 @@ public class NativeErrors { f = new File(path); if (!f.exists()) throw new RuntimeException("Cannot find native2ascii at "+path); + System.out.println("Using native2ascii at "+path); } return path; } diff --git a/jdk/test/sun/tools/native2ascii/test2 b/jdk/test/sun/tools/native2ascii/test2 deleted file mode 100644 index 9a669e9ced5..00000000000 --- a/jdk/test/sun/tools/native2ascii/test2 +++ /dev/null @@ -1 +0,0 @@ -This file exists as a non-writable placeholder for NativeErrors.java From f1fd7e28fe627b3b88bcca01737b71bd60f0aed9 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 18 Nov 2009 11:12:28 -0800 Subject: [PATCH 3/5] 6892742: Improve root set used by jhat Reviewed-by: tbell, dcubed --- .../classes/com/sun/tools/hat/internal/model/JavaStatic.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jdk/src/share/classes/com/sun/tools/hat/internal/model/JavaStatic.java b/jdk/src/share/classes/com/sun/tools/hat/internal/model/JavaStatic.java index 36f4e0e228d..86064914ea6 100644 --- a/jdk/src/share/classes/com/sun/tools/hat/internal/model/JavaStatic.java +++ b/jdk/src/share/classes/com/sun/tools/hat/internal/model/JavaStatic.java @@ -57,7 +57,10 @@ public class JavaStatic { id = ((JavaObjectRef)value).getId(); } value = value.dereference(snapshot, field); - if (value.isHeapAllocated()) { + if (value.isHeapAllocated() && + clazz.getLoader() == snapshot.getNullThing()) { + // static fields are only roots if they are in classes + // loaded by the root classloader. JavaHeapObject ho = (JavaHeapObject) value; String s = "Static reference from " + clazz.getName() + "." + field.getName(); From 8d62fe076c12c0ee824cf4cfb5ab3cdbd1d1d8bd Mon Sep 17 00:00:00 2001 From: Mandy Chung Date: Wed, 18 Nov 2009 22:29:16 -0800 Subject: [PATCH 4/5] 6902678: com.sun.tracing.ProviderFactory.createProvider doesn't throw IllegalArgumentException DoPrivileged for calls that have permission check instead of catching all exceptions Reviewed-by: kamg, dcubed --- .../com/sun/tracing/ProviderFactory.java | 46 +++--- .../sun/tracing/MultiplexProviderFactory.java | 9 +- .../sun/tracing/NullProviderFactory.java | 9 +- .../tracing/PrintStreamProviderFactory.java | 9 +- .../classes/sun/tracing/ProviderSkeleton.java | 10 +- .../tracing/dtrace/DTraceProviderFactory.java | 12 +- .../com/sun/tracing/BasicWithSecurityMgr.java | 149 ++++++++++++++++++ 7 files changed, 188 insertions(+), 56 deletions(-) create mode 100644 jdk/test/com/sun/tracing/BasicWithSecurityMgr.java diff --git a/jdk/src/share/classes/com/sun/tracing/ProviderFactory.java b/jdk/src/share/classes/com/sun/tracing/ProviderFactory.java index 768a6bebb8d..1a4f064ff51 100644 --- a/jdk/src/share/classes/com/sun/tracing/ProviderFactory.java +++ b/jdk/src/share/classes/com/sun/tracing/ProviderFactory.java @@ -4,7 +4,10 @@ package com.sun.tracing; import java.util.HashSet; import java.io.PrintStream; import java.lang.reflect.Field; -import java.util.logging.Logger; +import java.security.AccessController; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import sun.security.action.GetPropertyAction; import sun.tracing.NullProviderFactory; import sun.tracing.PrintStreamProviderFactory; @@ -52,23 +55,17 @@ public abstract class ProviderFactory { HashSet factories = new HashSet(); // Try to instantiate a DTraceProviderFactory - String prop = null; - try { prop = System.getProperty("com.sun.tracing.dtrace"); } - catch (java.security.AccessControlException e) { - Logger.getAnonymousLogger().fine( - "Cannot access property com.sun.tracing.dtrace"); - } + String prop = AccessController.doPrivileged( + new GetPropertyAction("com.sun.tracing.dtrace")); + if ( (prop == null || !prop.equals("disable")) && DTraceProviderFactory.isSupported() ) { factories.add(new DTraceProviderFactory()); } // Try to instantiate an output stream factory - try { prop = System.getProperty("sun.tracing.stream"); } - catch (java.security.AccessControlException e) { - Logger.getAnonymousLogger().fine( - "Cannot access property sun.tracing.stream"); - } + prop = AccessController.doPrivileged( + new GetPropertyAction("sun.tracing.stream")); if (prop != null) { for (String spec : prop.split(",")) { PrintStream ps = getPrintStreamFromSpec(spec); @@ -89,22 +86,29 @@ public abstract class ProviderFactory { } } - private static PrintStream getPrintStreamFromSpec(String spec) { + private static PrintStream getPrintStreamFromSpec(final String spec) { try { // spec is in the form of ., where is // a fully specified class name, and is a static member // in that class. The must be a 'PrintStream' or subtype // in order to be used. - int fieldpos = spec.lastIndexOf('.'); - Class cls = Class.forName(spec.substring(0, fieldpos)); - Field f = cls.getField(spec.substring(fieldpos + 1)); - Class fieldType = f.getType(); + final int fieldpos = spec.lastIndexOf('.'); + final Class cls = Class.forName(spec.substring(0, fieldpos)); + + Field f = AccessController.doPrivileged(new PrivilegedExceptionAction() { + public Field run() throws NoSuchFieldException { + return cls.getField(spec.substring(fieldpos + 1)); + } + }); + return (PrintStream)f.get(null); - } catch (Exception e) { - Logger.getAnonymousLogger().warning( - "Could not parse sun.tracing.stream property: " + e); + } catch (ClassNotFoundException e) { + throw new AssertionError(e); + } catch (IllegalAccessException e) { + throw new AssertionError(e); + } catch (PrivilegedActionException e) { + throw new AssertionError(e); } - return null; } } diff --git a/jdk/src/share/classes/sun/tracing/MultiplexProviderFactory.java b/jdk/src/share/classes/sun/tracing/MultiplexProviderFactory.java index 6ede31ec158..14c27c76d86 100644 --- a/jdk/src/share/classes/sun/tracing/MultiplexProviderFactory.java +++ b/jdk/src/share/classes/sun/tracing/MultiplexProviderFactory.java @@ -30,7 +30,6 @@ import java.lang.reflect.InvocationTargetException; import java.util.HashMap; import java.util.HashSet; import java.util.Set; -import java.util.logging.Logger; import com.sun.tracing.ProviderFactory; import com.sun.tracing.Provider; @@ -65,13 +64,7 @@ public class MultiplexProviderFactory extends ProviderFactory { providers.add(factory.createProvider(cls)); } MultiplexProvider provider = new MultiplexProvider(cls, providers); - try { - provider.init(); - } catch (Exception e) { - // Probably a permission problem (can't get declared members) - Logger.getAnonymousLogger().warning( - "Could not initialize tracing provider: " + e.getMessage()); - } + provider.init(); return provider.newProxyInstance(); } } diff --git a/jdk/src/share/classes/sun/tracing/NullProviderFactory.java b/jdk/src/share/classes/sun/tracing/NullProviderFactory.java index f567acabb32..5416fe35e53 100644 --- a/jdk/src/share/classes/sun/tracing/NullProviderFactory.java +++ b/jdk/src/share/classes/sun/tracing/NullProviderFactory.java @@ -26,7 +26,6 @@ package sun.tracing; import java.lang.reflect.Method; -import java.util.logging.Logger; import com.sun.tracing.ProviderFactory; import com.sun.tracing.Provider; @@ -53,13 +52,7 @@ public class NullProviderFactory extends ProviderFactory { */ public T createProvider(Class cls) { NullProvider provider = new NullProvider(cls); - try { - provider.init(); - } catch (Exception e) { - // Probably a permission problem (can't get declared members) - Logger.getAnonymousLogger().warning( - "Could not initialize tracing provider: " + e.getMessage()); - } + provider.init(); return provider.newProxyInstance(); } } diff --git a/jdk/src/share/classes/sun/tracing/PrintStreamProviderFactory.java b/jdk/src/share/classes/sun/tracing/PrintStreamProviderFactory.java index 53013041653..cca6bb58df5 100644 --- a/jdk/src/share/classes/sun/tracing/PrintStreamProviderFactory.java +++ b/jdk/src/share/classes/sun/tracing/PrintStreamProviderFactory.java @@ -28,7 +28,6 @@ package sun.tracing; import java.lang.reflect.Method; import java.io.PrintStream; import java.util.HashMap; -import java.util.logging.Logger; import com.sun.tracing.ProviderFactory; import com.sun.tracing.Provider; @@ -54,13 +53,7 @@ public class PrintStreamProviderFactory extends ProviderFactory { public T createProvider(Class cls) { PrintStreamProvider provider = new PrintStreamProvider(cls, stream); - try { - provider.init(); - } catch (Exception e) { - // Probably a permission problem (can't get declared members) - Logger.getAnonymousLogger().warning( - "Could not initialize tracing provider: " + e.getMessage()); - } + provider.init(); return provider.newProxyInstance(); } } diff --git a/jdk/src/share/classes/sun/tracing/ProviderSkeleton.java b/jdk/src/share/classes/sun/tracing/ProviderSkeleton.java index 0178fe46ed8..dd7cbb1976a 100644 --- a/jdk/src/share/classes/sun/tracing/ProviderSkeleton.java +++ b/jdk/src/share/classes/sun/tracing/ProviderSkeleton.java @@ -32,6 +32,8 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.AnnotatedElement; import java.lang.annotation.Annotation; import java.util.HashMap; +import java.security.AccessController; +import java.security.PrivilegedAction; import com.sun.tracing.Provider; import com.sun.tracing.Probe; @@ -99,7 +101,13 @@ public abstract class ProviderSkeleton implements InvocationHandler, Provider { * It is up to the factory implementations to call this after construction. */ public void init() { - for (Method m : providerType.getDeclaredMethods()) { + Method[] methods = AccessController.doPrivileged(new PrivilegedAction() { + public Method[] run() { + return providerType.getDeclaredMethods(); + } + }); + + for (Method m : methods) { if ( m.getReturnType() != Void.TYPE ) { throw new IllegalArgumentException( "Return value of method is not void"); diff --git a/jdk/src/share/classes/sun/tracing/dtrace/DTraceProviderFactory.java b/jdk/src/share/classes/sun/tracing/dtrace/DTraceProviderFactory.java index 3148f6f6db4..ce921d88eeb 100644 --- a/jdk/src/share/classes/sun/tracing/dtrace/DTraceProviderFactory.java +++ b/jdk/src/share/classes/sun/tracing/dtrace/DTraceProviderFactory.java @@ -29,7 +29,6 @@ import java.util.Map; import java.util.Set; import java.util.HashMap; import java.util.HashSet; -import java.util.logging.Logger; import java.security.Permission; import com.sun.tracing.ProviderFactory; @@ -80,15 +79,8 @@ public final class DTraceProviderFactory extends ProviderFactory { DTraceProvider jsdt = new DTraceProvider(cls); T proxy = jsdt.newProxyInstance(); jsdt.setProxy(proxy); - try { - jsdt.init(); - new Activation(jsdt.getModuleName(), new DTraceProvider[] { jsdt }); - } catch (Exception e) { - // Probably a permission problem (can't get declared members) - Logger.getAnonymousLogger().warning( - "Could not initialize tracing provider: " + e.getMessage()); - jsdt.dispose(); - } + jsdt.init(); + new Activation(jsdt.getModuleName(), new DTraceProvider[] { jsdt }); return proxy; } diff --git a/jdk/test/com/sun/tracing/BasicWithSecurityMgr.java b/jdk/test/com/sun/tracing/BasicWithSecurityMgr.java new file mode 100644 index 00000000000..547eb09445b --- /dev/null +++ b/jdk/test/com/sun/tracing/BasicWithSecurityMgr.java @@ -0,0 +1,149 @@ +/* + * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +/** + * @test + * @bug 6899605 + * @summary Basic unit test for tracing framework with security manager + * enabled + */ + +import com.sun.tracing.*; +import java.lang.reflect.Method; + +@ProviderName("NamedProvider") +interface BasicProvider extends Provider { + void plainProbe(); + void probeWithArgs(int a, float f, String s, Long l); + @ProbeName("namedProbe") void probeWithName(); + void overloadedProbe(); + void overloadedProbe(int i); +} + +interface InvalidProvider extends Provider { + int nonVoidProbe(); +} + +public class BasicWithSecurityMgr { + + public static ProviderFactory factory; + public static BasicProvider bp; + + public static void main(String[] args) throws Exception { + // enable security manager + System.setSecurityManager(new SecurityManager()); + + factory = ProviderFactory.getDefaultFactory(); + if (factory != null) { + bp = factory.createProvider(BasicProvider.class); + } + + testProviderFactory(); + testProbe(); + testProvider(); + } + + static void fail(String s) throws Exception { + throw new Exception(s); + } + + static void testProviderFactory() throws Exception { + if (factory == null) { + fail("ProviderFactory.getDefaultFactory: Did not create factory"); + } + if (bp == null) { + fail("ProviderFactory.createProvider: Did not create provider"); + } + try { + factory.createProvider(null); + fail("ProviderFactory.createProvider: Did not throw NPE for null"); + } catch (NullPointerException e) {} + + try { + factory.createProvider(InvalidProvider.class); + fail("Factory.createProvider: Should error with non-void probes"); + } catch (IllegalArgumentException e) {} + } + + public static void testProvider() throws Exception { + + // These just shouldn't throw any exeptions: + bp.plainProbe(); + bp.probeWithArgs(42, (float)3.14, "spam", new Long(2L)); + bp.probeWithArgs(42, (float)3.14, null, null); + bp.probeWithName(); + bp.overloadedProbe(); + bp.overloadedProbe(42); + + Method m = BasicProvider.class.getMethod("plainProbe"); + Probe p = bp.getProbe(m); + if (p == null) { + fail("Provider.getProbe: Did not return probe"); + } + + Method m2 = BasicWithSecurityMgr.class.getMethod("testProvider"); + p = bp.getProbe(m2); + if (p != null) { + fail("Provider.getProbe: Got probe with invalid spec"); + } + + bp.dispose(); + // These just shouldn't throw any exeptions: + bp.plainProbe(); + bp.probeWithArgs(42, (float)3.14, "spam", new Long(2L)); + bp.probeWithArgs(42, (float)3.14, null, null); + bp.probeWithName(); + bp.overloadedProbe(); + bp.overloadedProbe(42); + + if (bp.getProbe(m) != null) { + fail("Provider.getProbe: Should return null after dispose()"); + } + + bp.dispose(); // just to make sure nothing bad happens + } + + static void testProbe() throws Exception { + Method m = BasicProvider.class.getMethod("plainProbe"); + Probe p = bp.getProbe(m); + p.isEnabled(); // just make sure it doesn't do anything bad + p.trigger(); + + try { + p.trigger(0); + fail("Probe.trigger: too many arguments not caught"); + } catch (IllegalArgumentException e) {} + + p = bp.getProbe(BasicProvider.class.getMethod( + "probeWithArgs", int.class, float.class, String.class, Long.class)); + try { + p.trigger(); + fail("Probe.trigger: too few arguments not caught"); + } catch (IllegalArgumentException e) {} + + try { + p.trigger((float)3.14, (float)3.14, "", new Long(0L)); + fail("Probe.trigger: wrong type primitive arguments not caught"); + } catch (IllegalArgumentException e) {} + } +} From 4da1e67db39bcd15d729b116d6615827d527bbb6 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Collet Date: Fri, 20 Nov 2009 14:50:55 +0100 Subject: [PATCH 5/5] 6901170: HttpCookie parsing of version and max-age mis-handled Accept single quotes in cookies and better exception handling in CookieManager Reviewed-by: chegar --- jdk/src/share/classes/java/net/CookieManager.java | 13 ++++++++++++- jdk/src/share/classes/java/net/HttpCookie.java | 11 +++++++---- jdk/test/java/net/CookieHandler/TestHttpCookie.java | 5 ++++- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/jdk/src/share/classes/java/net/CookieManager.java b/jdk/src/share/classes/java/net/CookieManager.java index 57038fcdbf2..986daeff337 100644 --- a/jdk/src/share/classes/java/net/CookieManager.java +++ b/jdk/src/share/classes/java/net/CookieManager.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Collections; import java.util.Comparator; import java.io.IOException; +import sun.util.logging.PlatformLogger; /** * CookieManager provides a concrete implementation of {@link CookieHandler}, @@ -263,6 +264,7 @@ public class CookieManager extends CookieHandler if (cookieJar == null) return; + PlatformLogger logger = PlatformLogger.getLogger("java.net.CookieManager"); for (String headerKey : responseHeaders.keySet()) { // RFC 2965 3.2.2, key must be 'Set-Cookie2' // we also accept 'Set-Cookie' here for backward compatibility @@ -277,7 +279,16 @@ public class CookieManager extends CookieHandler for (String headerValue : responseHeaders.get(headerKey)) { try { - List cookies = HttpCookie.parse(headerValue); + List cookies; + try { + cookies = HttpCookie.parse(headerValue); + } catch (IllegalArgumentException e) { + // Bogus header, make an empty list and log the error + cookies = java.util.Collections.EMPTY_LIST; + if (logger.isLoggable(PlatformLogger.SEVERE)) { + logger.severe("Invalid cookie for " + uri + ": " + headerValue); + } + } for (HttpCookie cookie : cookies) { if (cookie.getPath() == null) { // If no path is specified, then by default diff --git a/jdk/src/share/classes/java/net/HttpCookie.java b/jdk/src/share/classes/java/net/HttpCookie.java index bd16d4f2723..be82cc3efbd 100644 --- a/jdk/src/share/classes/java/net/HttpCookie.java +++ b/jdk/src/share/classes/java/net/HttpCookie.java @@ -1036,7 +1036,7 @@ public final class HttpCookie implements Cloneable { int version = Integer.parseInt(attrValue); cookie.setVersion(version); } catch (NumberFormatException ignored) { - throw new IllegalArgumentException("Illegal cookie version attribute"); + // Just ignore bogus version, it will default to 0 or 1 } } }); @@ -1147,12 +1147,15 @@ public final class HttpCookie implements Cloneable { } private static String stripOffSurroundingQuote(String str) { - if (str != null && str.length() > 0 && + if (str != null && str.length() > 2 && str.charAt(0) == '"' && str.charAt(str.length() - 1) == '"') { return str.substring(1, str.length() - 1); - } else { - return str; } + if (str != null && str.length() > 2 && + str.charAt(0) == '\'' && str.charAt(str.length() - 1) == '\'') { + return str.substring(1, str.length() - 1); + } + return str; } private static boolean equalsIgnoreCase(String s, String t) { diff --git a/jdk/test/java/net/CookieHandler/TestHttpCookie.java b/jdk/test/java/net/CookieHandler/TestHttpCookie.java index f1f89f8daab..df0dd74cdb7 100644 --- a/jdk/test/java/net/CookieHandler/TestHttpCookie.java +++ b/jdk/test/java/net/CookieHandler/TestHttpCookie.java @@ -24,7 +24,7 @@ /** * @test * @summary Unit test for java.net.HttpCookie - * @bug 6244040 6277796 6277801 6277808 6294071 6692802 6790677 + * @bug 6244040 6277796 6277801 6277808 6294071 6692802 6790677 6901170 * @author Edward Wang */ @@ -335,6 +335,9 @@ public class TestHttpCookie { // bug 6277801 test("set-cookie: CUSTOMER=WILE_E_COYOTE; path=/; expires=Wednesday, 09-Nov-99 23:12:40 GMT; path=\"/acme\"") .n("CUSTOMER").v("WILE_E_COYOTE").p("/").ver(0); + + // bug 6901170 + test("set-cookie: CUSTOMER=WILE_E_COYOTE; version='1'").ver(1); } static void misc() {