Skip to content

Commit

Permalink
fix: handle faulty imports (#12793)
Browse files Browse the repository at this point in the history
Do not throw silently for incompatible
import paths. Fix comment removal
for JS files to accept ' as string.

fixes #12765
  • Loading branch information
caalador committed Jan 27, 2022
1 parent 0e0b08a commit d7744e2
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 11 deletions.
31 changes: 29 additions & 2 deletions flow-server/src/main/java/com/vaadin/flow/internal/StringUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,32 @@ public final class StringUtil {
* Comment parser state enumeration.
*/
private enum State {
NORMAL, IN_LINE_COMMENT, IN_BLOCK_COMMENT, IN_STRING
NORMAL, IN_LINE_COMMENT, IN_BLOCK_COMMENT, IN_STRING, IN_STRING_APOSTROPHE
}

/**
* Removes comments (block comments and line comments) from the JS code.
*
* @param code
* code to clean comments from
* @return the code with removed comments
*/
public final static String removeComments(String code) {
public static String removeComments(String code) {
return removeComments(code, false);
}

/**
* Removes comments (block comments and line comments) from the JS code.
*
* @param code
* code to clean comments from
* @param useStringApostrophe
* if {@code true} then ' is also considered a string and
* comments will not be considered inside it
* @return the code with removed comments
*/
public static String removeComments(String code,
boolean useStringApostrophe) {
State state = State.NORMAL;
StringBuilder result = new StringBuilder();
Map<String, Character> replacements = new HashMap<>();
Expand All @@ -65,6 +82,8 @@ public final static String removeComments(String code) {
result.append(character);
if (character.equals("\"")) {
state = State.IN_STRING;
} else if (useStringApostrophe && character.equals("'")) {
state = State.IN_STRING_APOSTROPHE;
}
}
break;
Expand All @@ -76,6 +95,14 @@ public final static String removeComments(String code) {
result.append(scanner.next());
}
break;
case IN_STRING_APOSTROPHE:
result.append(character);
if (character.equals("'")) {
state = State.NORMAL;
} else if (character.equals("\\") && scanner.hasNext()) {
result.append(scanner.next());
}
break;
case IN_LINE_COMMENT:
if (character.equals("\n")) {
result.append(character);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.nio.charset.MalformedInputException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -544,7 +545,10 @@ private void visitImportsRecursively(Path filePath, String path,
if (file == null && !importedPath.startsWith("./")) {
// In case such file doesn't exist it may be external: inside
// node_modules folder
file = getFile(getNodeModulesDir(), resolvedPath);
file = getFile(getNodeModulesDir(), importedPath);
if (!file.exists()) {
file = null;
}
resolvedPath = importedPath;
}
if (file == null) {
Expand Down Expand Up @@ -602,12 +606,19 @@ private String resolve(String importedPath, Path moduleFile, String path) {
String pathPrefix = moduleFile.toString();
pathPrefix = pathPrefix.substring(0,
pathPrefix.length() - path.length());
String resolvedPath = moduleFile.getParent().resolve(importedPath)
.toString();
if (resolvedPath.startsWith(pathPrefix)) {
resolvedPath = resolvedPath.substring(pathPrefix.length());
try {
String resolvedPath = moduleFile.getParent().resolve(importedPath)
.toString();
if (resolvedPath.startsWith(pathPrefix)) {
resolvedPath = resolvedPath.substring(pathPrefix.length());
}
return resolvedPath;
} catch (InvalidPathException ipe) {
getLogger().error("Invalid import '{}' in file '{}'", importedPath,
moduleFile);
getLogger().debug("Failed to resolve path.", ipe);
}
return resolvedPath;
return importedPath;
}

private String normalizePath(String path) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ List<String> getImportedPaths() {
* @return the code with removed comments
*/
String removeComments() {
return StringUtil.removeComments(content);
return StringUtil.removeComments(content, true);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,45 @@ public void commentRemoval_emojiInString_removalDoesnotThrowResultIsTheSame() {
@Test
public void removeComments_commentsWithAsterisksInside_commentIsRemoved() {
String result = StringUtil.removeComments("/* comment **/ ;");
Assert.assertEquals(result, " ;");
Assert.assertEquals(" ;", result);
}

}
@Test
public void removeJsComments_handlesApostropheAsInString() {
String httpImport = "import 'http://localhost:56445/files/transformed/@vaadin/vaadin-text-field/vaadin-text-field.js';";

Assert.assertEquals("Nothing shoiuld be removed for import", httpImport,
StringUtil.removeComments(httpImport, true));

String result = StringUtil.removeComments("/* comment **/ ;", true);
Assert.assertEquals(" ;", result);

String singleLineBlock = StringUtil.removeComments(
"return html`/* single line block comment*/`;", true);

Assert.assertEquals("return html``;", singleLineBlock);

String blockComment = StringUtil
.removeComments("return html`/* block with new lines\n"
+ "* still in my/their block */`;", true);
Assert.assertEquals("return html``;", blockComment);

String newLineSingleBlock = StringUtil
.removeComments("return html`/* not here \n*/`;", true);
Assert.assertEquals("return html``;", newLineSingleBlock);

String noComments = "<vaadin-text-field label=\"Nats Url(s)\" placeholder=\"nats://server:port\" id=\"natsUrlTxt\" style=\"width:100%\"></vaadin-text-field>`";
Assert.assertEquals(noComments,
StringUtil.removeComments(noComments, true));

String lineComment = StringUtil
.removeComments("return html`// this line comment\n`;", true);
Assert.assertEquals("return html`\n`;", lineComment);

String mixedComments = StringUtil.removeComments(
"return html`/* not here \n*/\nCode;// neither this\n"
+ "/* this should // be fine\n* to remove / */`;",
true);
Assert.assertEquals("return html`\nCode;\n`;", mixedComments);
}
}

0 comments on commit d7744e2

Please sign in to comment.