groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thibault Kruse <tibokr...@googlemail.com>
Subject Re: About the enhanced version of `Properties`, i.e. `GProperties`
Date Mon, 05 Nov 2018 00:13:43 GMT
I have mentioned that the current PR code is vulnerable to infinite
cycles, unbalanced curlies, nested curlies in the value, and curlies
in the value from an interpolated property. I am not sure if you were
considering testing those cases. I would recommend you to extract a
method interpolate(String, Map properties) to make unit testing (and
unit test reviews) easier. Then I'd recommend defining the expected
results for accessing each of the following keys, and write a unit
test:

err1 = {err1}
e0 =
e1 = x
e2 = {}
e3 = {{}}
e4 = {{{}}}
u0={{}
u1={}}
u0e0={{e0}
u0e1={{e1}
u0e1={{e2}
u0e1={{e3}
u0e1={{e4}
u1e0={e0}}
u1e1={e1}}
u1e1={e2}}
u1e1={e3}}
u1e1={e4}}
re0={e0}
re1={e1}
re2={e2}
re3={e3}
re4={e4}
ru0={u0}
ru1={u1}
r2e0={{e0}}
r3e0={{{e0}}}
r3e1={{{e1}}}
r3e2={{{e2}}}
r3e3={{{e3}}}
r3e4={{{e4}}}

If at any point you feel confused about what the result should be,
then this is how the users will feel, too.
Maybe using the "${}" syntax (like GStrings) instead may make things
easier with respect to writing, reading, parsing and escaping (Again,
commons-configuration might do that for a reason, same for GStrings).

Also Groovy (or GProperties users) might want to switch to
commons-configuration some time in the future (because it has tons of
great additional features), so staying syntax-compatible with them may
be a good idea. The same goes for using "include" instead of
"import.properties". Groovy and commons-configuration are both apache
projects, it may make sense to cooperate even in a limited sense.




On Mon, Nov 5, 2018 at 1:16 AM Daniel.Sun <sunlan@apache.org> wrote:
>
> Hi Thibault,
>
>       As quite a few people worry about the security issues, I have removed
> evaluating script feature.
>
>       Here is the latest implementation with some essential groovydoc:
> https://github.com/apache/groovy/blob/master/src/main/groovy/groovy/util/GProperties.groovy
>
>        And here is the related tests to show its usage in detail:
> https://github.com/apache/groovy/blob/master/src/test/groovy/util/GPropertiesTest.groovy
>
>
> Cheers,
> Daniel.Sun
>
>
>
> -----
> Daniel Sun
> Apache Groovy committer
> Blog: http://blog.sunlan.me
> Twitter: @daniel_sun
>
> --
> Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html

Mime
View raw message