From 4fd6a0f49f53b1c820f325f0d56389cf41ee47e7 Mon Sep 17 00:00:00 2001 From: jantje Date: Wed, 1 Jul 2020 14:02:26 +0200 Subject: [PATCH] bug 560330 remove \${ "to not resolve" functionality This change causes incompatibility for users using the \${ to not expand environment variables. Tested with sloeber (700+ projects) Change-Id: If327f055a41c309c475e17e0239a30e7518c3b23 Signed-off-by: jantje --- .../cdt/utils/CdtVariableResolverTest.java | 68 ++++++++++++------- .../cdtvariables/CdtVariableResolver.java | 44 ++++-------- 2 files changed, 60 insertions(+), 52 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/CdtVariableResolverTest.java b/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/CdtVariableResolverTest.java index 362336f1c6a..677885b97bc 100644 --- a/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/CdtVariableResolverTest.java +++ b/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/CdtVariableResolverTest.java @@ -13,6 +13,8 @@ *******************************************************************************/ package org.eclipse.cdt.utils; +import static org.junit.Assert.assertNotEquals; + import org.eclipse.cdt.core.cdtvariables.CdtVariableException; import org.eclipse.cdt.core.cdtvariables.ICdtVariableStatus; import org.eclipse.cdt.utils.cdtvariables.CdtVariableResolver; @@ -23,6 +25,7 @@ import junit.framework.TestCase; import junit.framework.TestSuite; public class CdtVariableResolverTest extends TestCase { + private static String acceptedChars = "\\<>&é\"'(§è!çà|@#^¨* []?./+,;:=~)"; public static Test suite() { return new TestSuite(CdtVariableResolverTest.class); @@ -35,12 +38,21 @@ public class CdtVariableResolverTest extends TestCase { if (macroName.equals("null")) { return null; } + if (macroName.equals("op")) { + return "op"; + } + if (macroName.equals("ro")) { + return "ro"; + } if (macroName.equals("loop")) { return "${LOOP}"; } if (macroName.equals("LOOP")) { return "${loop}"; } + if (macroName.equals(acceptedChars)) { + return "OK"; + } if (macroName.equals("throw")) { throw new CdtVariableException(ICdtVariableStatus.TYPE_MACRO_UNDEFINED, null, null, null); } @@ -64,50 +76,60 @@ public class CdtVariableResolverTest extends TestCase { private MockSubstitutor mockSubstitutor = new MockSubstitutor(); + //wrapper method to make code easier to read + private String resolveToString(String key) throws CdtVariableException { + return CdtVariableResolver.resolveToString(key, mockSubstitutor); + } + public void testResolveToString() throws CdtVariableException { - assertEquals("", CdtVariableResolver.resolveToString(null, mockSubstitutor)); - assertEquals("", CdtVariableResolver.resolveToString("", mockSubstitutor)); - assertEquals("Text", CdtVariableResolver.resolveToString("Text", mockSubstitutor)); - assertEquals("#Macro#", CdtVariableResolver.resolveToString("${Macro}", mockSubstitutor)); - assertEquals("", CdtVariableResolver.resolveToString("${}", mockSubstitutor)); - assertEquals("${Nomacro", CdtVariableResolver.resolveToString("${Nomacro", mockSubstitutor)); - assertEquals("Nomacro}", CdtVariableResolver.resolveToString("Nomacro}", mockSubstitutor)); - assertEquals("Text/#Macro#", CdtVariableResolver.resolveToString("Text/${Macro}", mockSubstitutor)); - assertEquals("#Macro#/Text", CdtVariableResolver.resolveToString("${Macro}/Text", mockSubstitutor)); - assertEquals("#Macro1#/#Macro2#", CdtVariableResolver.resolveToString("${Macro1}/${Macro2}", mockSubstitutor)); - assertEquals("${Macro}", CdtVariableResolver.resolveToString("\\${Macro}", mockSubstitutor)); - assertEquals("${Macro}:#Macro#", CdtVariableResolver.resolveToString("\\${Macro}:${Macro}", mockSubstitutor)); - assertEquals("\\#Macro#", CdtVariableResolver.resolveToString("\\\\${Macro}", mockSubstitutor)); - assertEquals("\\${Macro}", CdtVariableResolver.resolveToString("\\\\\\${Macro}", mockSubstitutor)); - assertEquals("C:\\tmp\\", CdtVariableResolver.resolveToString("C:\\tmp\\", mockSubstitutor)); + assertEquals("", resolveToString(null)); + assertEquals("", resolveToString("")); + assertEquals("Text", resolveToString("Text")); + assertEquals("#Macro#", resolveToString("${Macro}")); + assertEquals("", resolveToString("${}")); + assertEquals("${Nomacro", resolveToString("${Nomacro")); + assertEquals("Nomacro}", resolveToString("Nomacro}")); + assertEquals("Text/#Macro#", resolveToString("Text/${Macro}")); + assertEquals("#Macro#/Text", resolveToString("${Macro}/Text")); + assertEquals("#Macro1#/#Macro2#", resolveToString("${Macro1}/${Macro2}")); + assertEquals("#=Macro#", resolveToString("${=Macro}")); + assertEquals("#=Macro#:#Macro#", resolveToString("${=Macro}:${Macro}")); + assertEquals("\\#Macro#", resolveToString("\\${Macro}")); + assertEquals("\\#=Macro#", resolveToString("\\${=Macro}")); + assertEquals("Text/#=Macro#", resolveToString("Text/${=Macro}")); + assertEquals("Text/#=Macro#text", resolveToString("Text/${=Macro}text")); + assertEquals("Text/#Macro#text", resolveToString("Text/${Macro}text")); + assertEquals("Text/#Macro#text", resolveToString("Text/${Mac${ro}}text")); + assertEquals("C:\\tmp\\", resolveToString("C:\\tmp\\")); + assertEquals("OK", resolveToString("${" + acceptedChars + "}")); + //resolve should only resolve 1 level deep + assertNotEquals(resolveToString("${LOOP}"), resolveToString(resolveToString("${LOOP}"))); - assertEquals("#workspace_loc:#Macro##", - CdtVariableResolver.resolveToString("${workspace_loc:${Macro}}", mockSubstitutor)); - assertEquals("#workspace_loc:#Macro1#/#Macro2##", - CdtVariableResolver.resolveToString("${workspace_loc:${Macro1}/${Macro2}}", mockSubstitutor)); + assertEquals("#workspace_loc:#Macro##", resolveToString("${workspace_loc:${Macro}}")); + assertEquals("#workspace_loc:#Macro1#/#Macro2##", resolveToString("${workspace_loc:${Macro1}/${Macro2}}")); assertEquals("#workspace_loc:#project_loc:/#Macro###", - CdtVariableResolver.resolveToString("${workspace_loc:${project_loc:/${Macro}}}", mockSubstitutor)); + resolveToString("${workspace_loc:${project_loc:/${Macro}}}")); } public void testExceptions() throws CdtVariableException { // test exceptions try { - assertEquals("Unreacheable", CdtVariableResolver.resolveToString("${null}", mockSubstitutor)); + assertEquals("Unreacheable", resolveToString("${null}")); fail("Exception expected"); } catch (CdtVariableException e) { // expected behavior } try { - assertEquals("Unreacheable", CdtVariableResolver.resolveToString("${throw}", mockSubstitutor)); + assertEquals("Unreacheable", resolveToString("${throw}")); fail("Exception expected"); } catch (CdtVariableException e) { // expected behavior } // make sure there is no infinite loop - assertEquals("${LOOP}", CdtVariableResolver.resolveToString("${loop}", mockSubstitutor)); + assertEquals("${LOOP}", resolveToString("${loop}")); } public void testAsList() throws CdtVariableException { diff --git a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/cdtvariables/CdtVariableResolver.java b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/cdtvariables/CdtVariableResolver.java index 6c720512b5d..818f50f357b 100644 --- a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/cdtvariables/CdtVariableResolver.java +++ b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/cdtvariables/CdtVariableResolver.java @@ -62,17 +62,10 @@ public class CdtVariableResolver { private static final String EMPTY_STRING = ""; //$NON-NLS-1$ public static final String VARIABLE_PREFIX = "${"; //$NON-NLS-1$ - private static final String VARIABLE_PREFIX_MASKED = "$\1"; //$NON-NLS-1$ public static final char VARIABLE_SUFFIX = '}'; - private static final char VARIABLE_SUFFIX_MASKED = '\2'; - public static final char VARIABLE_ESCAPE_CHAR = '\\'; - private static final char VARIABLE_ESCAPE_CHAR_MASKED = '\3'; - - // Regular expression fragments private static final String RE_VPREFIX = "\\$\\{"; //$NON-NLS-1$ private static final String RE_VSUFFIX = "\\}"; //$NON-NLS-1$ private static final String RE_VNAME = "[^${}]*"; //$NON-NLS-1$ - private static final String RE_BSLASH = "[\\\\]"; // *one* backslash //$NON-NLS-1$ /** * Converts list of strings to one string using given string as delimiter, @@ -99,8 +92,9 @@ public class CdtVariableResolver { /** * Resolves macros of kind ${Macro} in the given string by calling the macro substitutor * for each macro reference found. Macros can be inside one another like - * ${workspace_loc:/${ProjName}/} but resolved just once. No recursive or concatenated - * macro names are allowed. It is possible to prevent macro from expanding using backslash \$. + * ${workspace_loc:/${ProjName}/} but resolved just once. No recursive + * macro names are allowed. + * It is not possible to prevent macros from expanding. * * @param string - macro expression. * @param substitutor - macro resolution provider to retrieve macro values. @@ -113,43 +107,35 @@ public class CdtVariableResolver { return EMPTY_STRING; } - final Pattern pattern = Pattern - .compile(".*?(" + RE_BSLASH + "*)(" + RE_VPREFIX + "(" + RE_VNAME + ")" + RE_VSUFFIX + ").*"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ + final Pattern pattern = Pattern.compile("(\\$\\{([^${}]*)\\})"); //$NON-NLS-1$ + final String VARIABLE_PREFIX_MASKED = "$\1"; //$NON-NLS-1$ + final char VARIABLE_SUFFIX_MASKED = '\2'; StringBuilder buffer = new StringBuilder(string); int limit = string.length(); - for (Matcher matcher = pattern.matcher(buffer); matcher.matches(); matcher = pattern.matcher(buffer)) { - String bSlashes = matcher.group(1); - String macro = matcher.group(2); - String name = matcher.group(3); + Matcher matcher = pattern.matcher(buffer); + while (matcher.find()) { + String name = matcher.group(2); String resolved = name.length() > 0 ? substitutor.resolveToString(name) : EMPTY_STRING; if (resolved == null) { throw new CdtVariableException(ICdtVariableStatus.TYPE_MACRO_UNDEFINED, null, string, name); } - if (limit-- < 0) { - // to prevent incidental looping - throw new CdtVariableException(ICdtVariableStatus.TYPE_ERROR, name, matcher.group(0), resolved); - } - int nBSlashes = bSlashes.length(); - if ((nBSlashes & 1) == 1) { - // if odd number of backslashes in front of "${...}" do not expand macro - resolved = macro; + if (limit-- < 0) { + // to prevent incidental endless looping + throw new CdtVariableException(ICdtVariableStatus.TYPE_ERROR, name, string, resolved); } // Only one expansion is allowed, so hide any text interfering with macro syntax resolved = resolved.replace(VARIABLE_PREFIX, VARIABLE_PREFIX_MASKED); resolved = resolved.replace(VARIABLE_SUFFIX, VARIABLE_SUFFIX_MASKED); - buffer.replace(matcher.start(2), matcher.end(2), resolved); - // collapse and hide backslashes \\\\${Macro} -> \\MacroValue or \\\\\${Macro} -> \\${Macro} - buffer.replace(matcher.start(1), matcher.end(1), - bSlashes.substring(0, nBSlashes / 2).replace(VARIABLE_ESCAPE_CHAR, VARIABLE_ESCAPE_CHAR_MASKED)); + + buffer.replace(matcher.start(1), matcher.end(1), resolved); + matcher = pattern.matcher(buffer); } String result = buffer.toString(); - // take hidden data back result = result.replace(VARIABLE_PREFIX_MASKED, VARIABLE_PREFIX); result = result.replace(VARIABLE_SUFFIX_MASKED, VARIABLE_SUFFIX); - result = result.replace(VARIABLE_ESCAPE_CHAR_MASKED, VARIABLE_ESCAPE_CHAR); return result; }