styx icon indicating copy to clipboard operation
styx copied to clipboard

Confusing message when placeholder tries to use value containing $

Open kvosper opened this issue 7 years ago • 0 comments

The problem

When PlaceholderResolver.replacePlaceholder is called with a placeholderValue that contains a $ (i.e. during configuration loading), it results in a confusing error message.

Detailed description

When PlaceholderResolver.replacePlaceholder is called with a placeholderValue that contains a $, the String.replaceAll method interprets it as referring to a matching group.

This results in a confusing error message:

  • for a non-numeric value after $ we get java.lang.IllegalArgumentException: Illegal group reference
  • for a numeric value after $ we get something akin to java.lang.IndexOutOfBoundsException: No group 1

Our options are to either escape the symbol or continue to fail, but with a better error message:

  • Pro of escaping: value is allowed through to Styx classes, which can decide whether to allow it or not on a case-by-case basis
  • Con of escaping: when it is injected into a value that should be a non-string (e.g.), Jackson will provide the error, which may be out-of-context
  • Con of escaping: multiple points of failure

Acceptance criteria

  • Determine whether values containing $ should be accepted
  • If we accept them, escape the symbol so that it does not cause an exception
  • If we do not, throw an exception with a message containing the placeholder and replacement

kvosper avatar Jan 08 '18 17:01 kvosper