StringUtil#split omits backslashes in property values

[Pepo48]

After we replaced the deprecated quarkus.http.ssl.certificate.file and quarkus.http.ssl.certificate.key-file properties with the new plural form counterparts, a Windows specific issue report was filed in the Keycloak repository.

In the case of ssl-certificate-*files properties Keycloak just consumes the Quarkus properties and exposes them further as is, there is no additional logic, nor validation. This makes me believe that this rather bug on Quarkus/SmallRye side.

I actually debugged the issue and I tracked it down to the StringUtil's split method. The Windows platform behaviour is following: As you can see, the way the matcher pattern is designed now, it omits backslashes, which then results to the NoSuchFile Keycloak issue mentioned above.

How to reproduce? The following demo test can be added to StringUtilTest.java:

void mixedWindowsAndUnixPath() {
    String text = "C:\\Program Files\\quarkus/extensions/example.java";
    assertEquals("C:/Program Files/quarkus/extensions/example.java", StringUtil.split(text)[0]);
}

Solution Either we can preemptively normalize path values in a way converted = converter.convert(configValue.getValue().replace("\\,", "/"));. Alternatively we can adjust the pattern in StringUtil.

Or we can workaround this on the Keycloak side (I already have a proposal) in case that this won't be considered as a bug.

Let me know your thoughts. I can definitely help with providing a PR.

Thanks.

[dmlloyd]

The backslash character acts as an escape character for delimiting list items in the list converter, to allow items to contain commas. If you want a literal backslash then you will need another layer of escapes (that is, four backslashes instead of two).

[radcortez]

Also, please check https://github.com/smallrye/smallrye-config/blob/b9ad8b3369ffac72c01bdc6e7275672e0d718079/common/src/test/java/io/smallrye/config/common/utils/StringUtilTest.java#L28 for some examples.

[Pepo48]

Thanks guys for the prompt replies.

Should then ExpressionConfigSourceInterceptor respect these rules? I probably didn't stress it enough, but the original Keycloak issue is related to env variable expansion. The ExpressionConfigSourceInterceptor seems to return value in the following format: with two backslashes, which then StringUtils wrongly considers to be a delimiter.

According to what you just mentioned above, shouldn't the expansion instead of the C:\\Users\\hudson\\IdeaProjects\\keycloak\\quarkus\\server\\target/kc produce C:\\\\Users\\\\hudson\\\\IdeaProjects\\\\keycloak\\\\quarkus\\\\server\\\\target/kc or simply the Unix way (modern Windows systems can understand the format)C:/Users/hudson/IdeaProjects/keycloak/quarkus/server/target/kc? Or am I still missing something?

Thanks for your time.

[dmlloyd]

The properties expander doesn't know anything about list conversion unfortunately; list expansion with special escapes is a quirk of MicroProfile Config which introduces several edge cases like this. Nor does the properties expander know that the thing you're substituting is a file name (it treats it as an arbitrary string).

A good enhancement might be to provide some kind of property expansion modifier character which adds a level of escapes to the expanded expression (something like ${escape::${sub.expression}} or similar), if that's even possible (I'm not sure whether it is). This proposed mechanism could replace \ with \\ in the substituted string.

[Pepo48]

The question is whether you guys consider this as a valid issue/enhancement that deserves more attention? If not, we can probably proceed with an workaround on our end (although we typically try to avoid it if possible).

Purely from the Keycloak perspective it has an importance already, since Keycloak is commonly used on the Windows platform.

[radcortez]

Feel free to send a PR, or I can have a look when I find some time.

[dmlloyd]

Another possible solution might be to (optionally?) change the nature of property expansion so that it is done as a part of the conversion process. This might allow the list converter to expand the list before properties are replaced. A negative effect of this solution is that it would prevent property expansion from being able to expand into a list value with embedded separators: in other words, foo.list=${baz}/baz=a,b,c,d for example would create a list with one item instead of a list of four items.

[radcortez]

@Pepo48 btw, how are you retrieving the configuration? programmatically? with a mapping? an alternative could be to provide your own customer Converter.