Escaping backslash does not work in front of replacement expression
I can't find a way to evaluate successfully something like x\${x} to x\foo.
With config:
x: foo
a: x/${x}
b: x\${x}
c: x\${x}
d: x\\${x}
e: x\\\${x}
f: x\\\\${x}
and code:
@ConfigProperty(name = "x") String x;
@ConfigProperty(name = "a") String a;
@ConfigProperty(name = "b") String b;
@ConfigProperty(name = "c") String c;
@ConfigProperty(name = "d") String d;
@ConfigProperty(name = "e") String e;
@ConfigProperty(name = "f") String f;
@Test
public void x() {
log.info("x=" + x);
log.info("a=" + a);
log.info("b=" + b);
log.info("c=" + c);
log.info("d=" + d);
log.info("e=" + e);
log.info("f=" + f);
}
We get:
x=foo
a=x/foo
b=x${x}
c=x${x}
d=x\${x}
e=x\\${x}
f=x\\\${x}
The \ seems to escape the $, resulting in ${x} not being evaluated.
But then I can't escape the escape character.
/cc @radcortez
This may actually be a bug in smallrye/smallrye-common's expression module string parser.
For now, you can work around this by adding an expression to the backslash:
endpoint=foo${backslash}${path}
backslash=\\
path=bar
~~The expression ${\} should also work to produce a literal backslash.~~ No, that's incorrect; sorry. I'm reading the code and I was in the ESCAPES section when I thought I was somewhere else.
I wonder if the config parser did not set the ESCAPES flag to the parser?
The Expression code works correctly and as expected. The main issue was when MP Config added support for Property Expressions; it defined a specific escape sequence to skip the expansion with \\$. If I remember correctly, my original proposal was to just support double $$(which is our way to escape it) but that was rejected by the group.
So, to comply with MP Config I've added this:
https://github.com/smallrye/smallrye-config/blob/9fdb499e168b46f6031b8af7b8fef309d8fc6101/implementation/src/main/java/io/smallrye/config/ExpressionConfigSourceInterceptor.java#L71-L92
Which turns the \\$ into our espace sequence $$, so that is why you don't get the expansion working in that case.
Turning on the Expression.Flag#ESCAPES introduced other issues, observed here now https://github.com/smallrye/smallrye-config/pull/747, so we opted out of it to not break users. If I remember correctly, when I tried to turn it on, it was causing issues with the PATH variable.
So at this point, I'm not totally sure what to do. Either we add another escape sequence for the \\$or we ignore MP Config (which may break other users).
@dmlloyd what do you think?
In this regard (as in many others) I think we can see that MP Config is fundamentally broken. The caution/lesson here (which, as you know, I've stated repeatedly to the Jakarta Config spec group) is that when designing a specification with syntax, the syntax has to be clearly specified - at least more strongly than "let's put in a random solution without thinking too deeply about the potential problems". 😞
The potential problem in this case is that with \$ escaping $ but \ otherwise not being an escape, we've created a syntax issue. How does one express a string that contains the sequence \$?
In the end we need a syntax grammar which allows:
- Every possible string to be expressed, somehow, and
- The inconsistent/ill-considered syntax rules of MP Config to somehow still work
while hopefully accounting for Jakarta Config. It almost doesn't seem worth it.
In my opinion I think we're better off having a consistent syntax that we can support, and just documenting the differences with MP Config and giving up on spec compliance altogether (unless the spec in question actually makes sense).
I agree. I probably should have opposed this harder, but you know how things work.
I don't think this is used extensively. Of course, there is a chance to break someone with unpredictable results, but I see no other consistent way to do it.
I wouldn't worry about Jakarta Config having to support MP Config syntax. This will be the least of the problems with all the discussions and how the spec is moving in Jakarta Config.
and giving up on spec compliance altogether (unless the spec in question actually makes sense).
at least temporarily, and hopefully get a way to change the spec.
I think we should be able to push a change to MP Config.
@dmlloyd What do you think we propose to change it with the original implementation support of the double $ to escape expansion?
I'd have to review the syntax rules again before having an informed opinion, but I think it's a reasonable idea. My feeling is that \$ is likely to still be a problem though. We'd need to specify what \\$ and \$$ should do, at the least.
Sure.
I've found this comment from me:
https://github.com/eclipse/microprofile-config/pull/577#issuecomment-659502523
I guess that was not enough, but I'm also to blame here 👿