RFR: 8185855: Debug exception stacks should be clearer

Seán Coffey sean.coffey at oracle.com
Wed Dec 6 10:44:06 UTC 2017


whoops - pasted the wrong file diffs in last email :

diff --git 
a/src/java.base/share/classes/sun/security/util/AnchorCertificates.java 
b/src/java.base/share/classes/sun/security/util/AnchorCertificates.java
--- a/src/java.base/share/classes/sun/security/util/AnchorCertificates.java
+++ b/src/java.base/share/classes/sun/security/util/AnchorCertificates.java
@@ -75,8 +75,8 @@
                  } catch (Exception e) {
                      if (debug != null) {
                          debug.println("Error parsing cacerts");
+                        e.printStackTrace();
                      }
-                    e.printStackTrace();
                  }
                  return null;
              }

Regards,
Sean.

On 06/12/17 10:20, Seán Coffey wrote:
> Thanks for reviewing Sean. Not finding the cacerts file has always 
> resulted in the stacktrace going to stderr in the past. I guess 
> there's no harm putting it behind a debug flag. New diff for that file 
> below. I'll push later today if I hear nothing else.
>
> diff --git 
> a/src/java.base/share/classes/sun/security/provider/AuthPolicyFile.java 
> b/src/java.base/share/classes/sun/security/provider/AuthPolicyFile.java
> --- 
> a/src/java.base/share/classes/sun/security/provider/AuthPolicyFile.java
> +++ 
> b/src/java.base/share/classes/sun/security/provider/AuthPolicyFile.java
> @@ -187,6 +187,7 @@
>              } catch (Exception e) {
>                  // ignore, treat it like we have no keystore
>                  if (debug != null) {
> +                    debug.println("Debug info only. No keystore.");
>                      e.printStackTrace();
>                  }
>                  return null;
> @@ -261,7 +262,7 @@
>                  loaded_one = true;
>              } catch (Exception e) {
>                  if (debug != null) {
> -                    debug.println("error reading policy " + e);
> +                    debug.println("Debug info only. Error reading 
> policy " + e);
>                      e.printStackTrace();
>                  }
>                  // ignore that policy
>
> Regards,
> Sean.
>
> On 05/12/17 20:41, Sean Mullan wrote:
>> On 12/5/17 9:27 AM, Seán Coffey wrote:
>>> Looking to improve the stacktrace output made when debug mode is 
>>> enabled for java.security and sun.security classes. In the past, 
>>> some of these have led to confusion for end users. Best to add some 
>>> context when we're printing stacktrace for informational, debug, 
>>> purposes.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8185855
>>>
>>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8185855/webrev/
>>>
>>> The one minor functional change I've made is in 
>>> sun/security/util/AnchorCertificates.java
>>> I figured that it's best to print some context around a stacktrace 
>>> that we're about to print rather than print it only in debug mode.
>>
>> I would probably put the e.printStackTrace inside the debug, seems 
>> odd to have the trace dumped to stderr even if debug is not set.
>>
>> Looks fine otherwise.
>>
>> --Sean
>




More information about the security-dev mailing list