Skip to content

Conversation

@timdp
Copy link

@timdp timdp commented Sep 5, 2024

Currently, StringEscapeUtils unescapes ECMAScript using the Java strategy.

This mostly works, but ECMAScript additionally supports \xCC escapes, where CC is a hex code.

This PR adds support for that, as well as an initial test.

I took the liberty of making the child translators of the Java aggregate translator reusable, so I could basically extend the list for the ES version. If we prefer to duplicate that code or to use a varargs list or something, I'm absolutely fine with that as well.

I just subscribed to the dev list and requested access to Jira. I will update the PR title once I've created the corresponding issue.

@timdp
Copy link
Author

timdp commented Sep 5, 2024

My Jira account request got denied and I have a low tolerance for red tape, so I'll just leave this PR here for anyone who cares enough to get it merged.

@garydgregory
Copy link
Member

Hello @timdp
Please run 'mvn' by itself to catch all build failures.

*/
public static final CharSequenceTranslator UNESCAPE_ECMASCRIPT = UNESCAPE_JAVA;
public static final CharSequenceTranslator UNESCAPE_ECMASCRIPT = new AggregateTranslator(
new HexUnescaper(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency should be a static constant as well?

final int value = Integer.parseInt(hex.toString(), 16);
writer.write((char) value);
} catch (final NumberFormatException nfe) {
throw new IllegalArgumentException("Unable to parse ASCII value: " + hex, nfe);
Copy link
Contributor

@ecki ecki Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need to document the IAE for the unescape functions and translator objects. An Abort is valid, it is a Syntax error in ECMAScript (need to verify the same for Java)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants