I tend to agree with you but it does get a little tricky. If "my own
user code" is making use of e.g. Java collection classes, then
sometimes I'd really like to see "one level down".
Take this example:
///////////////////////////////////////
def list = [null, 'a', 'b', 'c', 'd', 'e', 'f']
assert list.size() == 7
def bc = ['b', 'c']
list.removeAll(bc)
assert list.size() == 5
def de = ([d:1, e:2] as Hashtable).keySet()
list.removeAll(de) // <= NPE
///////////////////////////////////////
Here I have Lists and Maps as part of my user code. In some sense the
fact that they are "java.util.ArrrayList" and Hashtable should be
irrelevant but sometimes I also need to know.
With full stack traces we get:
Caught: java.lang.NullPointerException
java.lang.NullPointerException
at java.util.Hashtable.containsKey(Hashtable.java:336)
at java.util.Hashtable$KeySet.contains(Hashtable.java:654)
at
java.util.Collections$SynchronizedCollection.contains(Collections.java:2023)
at java.util.ArrayList.batchRemove(ArrayList.java:726)
at java.util.ArrayList.removeAll(ArrayList.java:696)
at
org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:318)
at Script.run(Script.groovy:9)
at
groovy.lang.GroovyShell.runScriptOrMainOrTestOrRunnable(GroovyShell.java:287)
at groovy.lang.GroovyShell.run(GroovyShell.java:393)
at groovy.lang.GroovyShell.run(GroovyShell.java:382)
at groovy.ui.GroovyMain.processOnce(GroovyMain.java:649)
at groovy.ui.GroovyMain.run(GroovyMain.java:389)
at groovy.ui.GroovyMain.access$1400(GroovyMain.java:67)
at groovy.ui.GroovyMain$GroovyCommand.process(GroovyMain.java:313)
at groovy.ui.GroovyMain.processArgs(GroovyMain.java:141)
at groovy.ui.GroovyMain.main(GroovyMain.java:114)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.codehaus.groovy.tools.GroovyStarter.rootLoader(GroovyStarter.java:109)
at org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:132)
Which I agree, I'd rarely want!!
With our current sanitized list we get:
Caught: java.lang.NullPointerException
java.lang.NullPointerException
at Script.run(Script.groovy:9)
If we instead defaulted to:
org.codehaus.groovy. org.apache.groovy. java.lang.reflect sun.
gjdk.groovy. groovy. jdk.internal
Then we'd get:
Caught: java.lang.NullPointerException
java.lang.NullPointerException
at java.util.Hashtable.containsKey(Hashtable.java:336)
at java.util.Hashtable$KeySet.contains(Hashtable.java:654)
at
java.util.Collections$SynchronizedCollection.contains(Collections.java:2023)
at java.util.ArrayList.batchRemove(ArrayList.java:726)
at java.util.ArrayList.removeAll(ArrayList.java:696)
at Script.run(Script.groovy:9)
And from this I can look in the Javadoc for ArrayList#removeAll and see:
> @throws NullPointerException if this list contains a null element and the
> specified collection does not permit null elements
And I could look at the Hashtable Javadoc and indeed see that it
doesn't permit nulls in its keys.
Now, what I'd probably like more is something like:
Caught: java.lang.NullPointerException
java.lang.NullPointerException
at java.util.ArrayList.removeAll(ArrayList.java:696)
at Script.run(Script.groovy:9)
But that's not what our current sanitizer supports.
Cheers, Paul.
On Thu, Jun 23, 2022 at 4:34 PM Guillaume Laforge <glafo...@gmail.com> wrote:
>
> For me, sanitization is essentially to hide all the plumbing, and only keep
> my own user code on the surface, so that I know where in my code it started
> failing.
> So I tend to want more sanitization than less :-)
>
> On Thu, Jun 23, 2022 at 3:29 AM Paul King <pa...@asert.com.au> wrote:
>>
>> Hi folks,
>>
>> We've had a recent PR to add "jdk.internal" to the list of packages we
>> remove when sanitizing stacktraces:
>>
>> https://github.com/apache/groovy/pull/1727
>>
>> We've also had a much earlier request to filter less aggressively, see:
>>
>> https://github.com/apache/groovy/pull/256
>> https://issues.apache.org/jira/browse/GROOVY-7756
>>
>> PR#1727 looks good to me but it seems worthwhile to consider both
>> requests if we are going to make a change.
>>
>> Current list: groovy., org.codehaus.groovy., java., javax., sun.,
>> gjdk.groovy.
>> Suggested by #1727: groovy., org.codehaus.groovy., java., javax.,
>> sun., gjdk.groovy., jdk.internal
>> Suggested by #256: groovy., org.codehaus.groovy., java.lang.reflect.,
>> sun.reflect., gjdk.groovy.
>>
>> If we were going to include (most of) "java", then there is an
>> argument that you should include "groovy" as well. My current thinking
>> is something like below might work:
>>
>> Suggestion: org.codehaus.groovy., org.apache.groovy,
>> java.lang.reflect, sun., gjdk.groovy., jdk.internal.
>>
>> The original thinking behind excluding "java." was that if you had a
>> script with e.g. Strings and Lists, you didn't really need to know
>> whether you had Java implemented data types or something supplied by
>> Groovy. While this is still a worthwhile goal, in reality most folks
>> need to know those datatypes pretty quickly, so hiding them away isn't
>> super beneficial.
>>
>> I also discovered that Grails uses a list something like this:
>> "org.codehaus.groovy.runtime.", "org.codehaus.groovy.reflection.",
>> "org.codehaus.groovy.ast.",
>> "org.springframework.web.filter", "org.springframework.boot.actuate",
>> "org.mortbay.",
>> "groovy.lang.", "org.apache.catalina.", "org.apache.coyote.",
>> "org.apache.tomcat.",
>> "net.sf.cglib.proxy.", "sun.", "java.lang.reflect.",
>> "org.springframework.boot.devtools.",
>> "org.springsource.loaded.", "com.opensymphony.", "javax.servlet."
>>
>> In terms of branches, I was thinking of Groovy 5 only and probably
>> back-ported to 4.
>>
>> And just a reminder, there is a "groovy.sanitized.stacktraces" system
>> property, so folks can change this if they really want.
>>
>> What do others think?
>>
>> Cheers, Paul.
>
>
>
> --
> Guillaume Laforge
> Apache Groovy committer
> Developer Advocate @ Google Cloud Platform
>
> Blog: http://glaforge.appspot.com/
> Twitter: @glaforge
