groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From MG <mg...@arscreat.com>
Subject Re: About the enhanced version of `Properties`, i.e. `GProperties`
Date Sat, 03 Nov 2018 17:07:38 GMT
Just a quick 0.9... Euro cents:

 1. Without having looked at the proposal in too much detail, I feel you
    raise some valid points.
 2. With regards to backwards compatibility, I feel we should not be too
    worried, since afaiu the differing file extension should keep these
    problems to a minimum.
 3. What I am wondering is, whether the approach would introduce all
    kinds of potential ways to inject malicious code into the running
    Groovy code, should someone manage to modify the gproperties file,
    for users like me who roll out compiled jars/class files, not Groovy
    scripts ? Maybe unfounded, the scenario just reminds me of the often
    used attack vector that seemingly harmless configuration/etc files
    execute stuff...

Cheers,
mg


Am 03.11.2018 um 15:30 schrieb Thibault Kruse:
> Hi all,
>
> thanks Daniel for making the suggestion and PR.
>
> BTW there already  is an extension in
> http://commons.apache.org/proper/commons-configuration/userguide/howto_properties.html#Using_PropertiesConfiguration
> including a syntax to include other properties files, to be considered
> (though that one allows duplicate keys, apparently), also it uses the
> ${} syntax to load environment variables, also considered a useful
> concept for property files by some people. See
> https://www.javacodegeeks.com/2015/04/a-way-to-read-properties-with-variable-interpolation.html
>
> I am also sceptical about adding this to groovy core to handle properties files.
>
> First let's document the specification of the idea: What you suggest
> is a solution that loads a property file using the JVM Properties
> class, and then on lookup of keys, loads the value as usual, but then
> applies some interpolations of marked substrings.
>
> I feel as a specification this is not precise enough, and I am not
> sure which behaviors of the PR are intended and which ones are
> accidental (see my review comments below).
>
> For usability my concern is that when wrapping value lookup by
> GProperties, a human reader cannot see that in the properties file,
> and so users are left confused as to what happens to any given
> properties file. A program could use Properties for one x.properties
> file, GProperties for y.properties file, and users of an application
> or library can only find out by trial and error which one is used for
> which.
> If any application or library replaced Properties with GProperties, I
> believe the results may be disastrous, because upgrading users would
> face any number of runtime exceptions due to new semantics of their
> values. It seems there is no backwards compatible upgrade path that
> guarantees to users that whatever values they have in the properties
> ile, switching to GProperties will maintain those values. Just as a
> brainstroming example: if only properties written such as "foo ~= {x}"
> would be interpolated, that would be a safe upgrade path (but I
> realize the coding would not be as simple as the current PR).
>
> There are also several edge and error cases currently not considered
> in the PR, not sure if the PR is just meant as a prototype, but let me
> list what I can think of so far:
> * infinite cycles in the importing of other properties
> * if a properties file imports another one, should the other one be
> loaded as Properties file or as GProperties file? Can the user decide?
> (the other file may come from a different jar outside the control of
> the user).
> * infinite cycles in the interpolation of variables
> * multiple interpolation when the substitution of a substring contains
> a pattern to be interpolated by itself: a=1; b={a}; c=${'{b}'}
> * nested interpolations "{foo${key}}"
> * unbalanced parentheses '{{key}'
> * if we have
> # parent.properties
> other=parent-other
> takeother={other}
>
> # child.properties
> import.properties=parent.properties
> other=child-other
> local={takeother}
>
> Which value will get(takeOther) in child.properties produce, and which
> should it produce? What if we involve more files with import
> relationships? Suddenly invisible links start creeping up between all
> imports.
>
>
> Some of the problems with the PR may be solvable (such as by using a
> single Regex instead of three consecutive ones), but a lot more
> test-cases are required, and the semantics of the import statement
> remain quite complicated to maintain, and for users to debug. Before
> including, I feel much more edge and error case testing should be done
> and a more precise specification should be written first.
>
> I don't want to generally discourage the idea, I hope my comments can
> help others come up with more concerns, ideas or alternatives.
>


Mime
View raw message