Skip to content

Commit

Permalink
fix: prevent embedded styles to leak main document (#19242) (#19249)
Browse files Browse the repository at this point in the history
When using an exported a themed Flow web-component, Lumo style may leak
the embedding document, causing invalid CSS rules to be applied.
This change prevents applying Lumo global imports when the theme is
applied to a web-component.

Fixes #12704

Co-authored-by: Marco Collovati <marco@vaadin.com>
  • Loading branch information
vaadin-bot and mcollovati committed Apr 26, 2024
1 parent 2eed13e commit 1c25fcf
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,17 @@ public void run() {

protected abstract void writeImportLines(List<String> lines);

// Visible for test
List<String> filterWebComponentImports(List<String> lines) {
if (lines != null) {
// Exclude Lumo global imports for exported web-component
return lines.stream()
.filter(line -> !line.contains("/lumo-includes.ts"))
.collect(Collectors.toList());
}
return lines;
}

/**
* Get all ES6 modules needed for run the application. Modules that are
* theme dependencies are guaranteed to precede other modules in the result.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -189,6 +188,12 @@ public class FrontendUtils {
*/
public static final String IMPORTS_D_TS_NAME = "generated-flow-imports.d.ts";

/**
* Name of the file that contains application imports, javascript, theme and
* style annotations used when embedding Flow as web-component.
*/
public static final String IMPORTS_WEB_COMPONENT_NAME = "generated-flow-webcomponent-imports.js";

public static final String THEME_IMPORTS_D_TS_NAME = "theme.d.ts";
public static final String THEME_IMPORTS_NAME = "theme.js";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.vaadin.flow.server.frontend.scanner.FrontendDependenciesScanner;

import static com.vaadin.flow.server.frontend.FrontendUtils.IMPORTS_NAME;
import static com.vaadin.flow.server.frontend.FrontendUtils.IMPORTS_WEB_COMPONENT_NAME;

/**
* An executor that it's run when the servlet context is initialised in dev-mode
Expand Down Expand Up @@ -122,7 +123,8 @@ public NodeTasks(Options options) {
options.frontendDirectory));
commands.add(new TaskGenerateWebComponentBootstrap(
options.frontendDirectory,
new File(options.generatedFolder, IMPORTS_NAME)));
new File(options.generatedFolder,
IMPORTS_WEB_COMPONENT_NAME)));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@

import static com.vaadin.flow.server.frontend.FrontendUtils.IMPORTS_D_TS_NAME;
import static com.vaadin.flow.server.frontend.FrontendUtils.IMPORTS_NAME;
import static com.vaadin.flow.server.frontend.FrontendUtils.IMPORTS_WEB_COMPONENT_NAME;

/**
* An updater that it's run when the servlet context is initialised in dev-mode
Expand Down Expand Up @@ -86,6 +87,7 @@ private class UpdateMainImportsFile extends AbstractUpdateImports {

private final File generatedFlowImports;
private final File generatedFlowDefinitions;
final File generatedFlowWebComponentImports;
private final File fallBackImports;
private final ClassFinder finder;

Expand All @@ -99,6 +101,8 @@ private class UpdateMainImportsFile extends AbstractUpdateImports {
generatedFlowImports = new File(generatedDirectory, IMPORTS_NAME);
generatedFlowDefinitions = new File(generatedDirectory,
IMPORTS_D_TS_NAME);
generatedFlowWebComponentImports = new File(generatedDirectory,
IMPORTS_WEB_COMPONENT_NAME);
finder = classFinder;
this.fallBackImports = fallBackImports;
}
Expand Down Expand Up @@ -137,6 +141,8 @@ protected void writeImportLines(List<String> lines) {
updateImportsFile(generatedFlowImports, lines);
updateImportsFile(generatedFlowDefinitions,
getDefinitionLines());
updateImportsFile(generatedFlowWebComponentImports,
filterWebComponentImports(lines));
} catch (IOException e) {
throw new IllegalStateException(String.format(
"Failed to update the Flow imports file '%s'",
Expand Down Expand Up @@ -202,6 +208,7 @@ protected URL getResource(String name) {
protected Collection<String> getGeneratedModules() {
final Set<String> exclude = new HashSet<>(
Arrays.asList(generatedFlowImports.getName(),
generatedFlowWebComponentImports.getName(),
FrontendUtils.FALLBACK_IMPORTS_NAME));
return NodeUpdater.getGeneratedModules(generatedFolder, exclude);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.function.Predicate;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -102,6 +103,7 @@ private class UpdateImports extends AbstractUpdateImports {
private final ClassFinder finder;
private final FrontendDependenciesScanner scanner;
private List<String> resultingLines;
private List<String> webcomponentImports;

UpdateImports(ClassFinder classFinder,
FrontendDependenciesScanner scanner, File npmDirectory,
Expand All @@ -116,6 +118,7 @@ private class UpdateImports extends AbstractUpdateImports {
@Override
protected void writeImportLines(List<String> lines) {
resultingLines = lines;
webcomponentImports = filterWebComponentImports(lines);
}

@Override
Expand Down Expand Up @@ -410,7 +413,32 @@ public void generate_containsLumoThemeFiles() {
"@vaadin/vaadin-lumo-styles/sizing.js",
"@vaadin/vaadin-lumo-styles/spacing.js",
"@vaadin/vaadin-lumo-styles/style.js",
"@vaadin/vaadin-lumo-styles/icons.js");
"@vaadin/vaadin-lumo-styles/icons.js", "./lumo-includes.ts");
}

@Test
public void generate_embeddedImports_doNotContainLumoGlobalThemeFiles()
throws IOException {
updater.run();

Predicate<String> containsLumoGlobalImports = line -> line
.contains("lumo-includes.ts");

List<String> flowImports = new ArrayList<>(updater.resultingLines);
assertTrue("Flow import should contain Lumo global import",
flowImports.stream().anyMatch(containsLumoGlobalImports));

assertTrue(
"Import for web-components should not contain Lumo global imports",
updater.webcomponentImports.stream()
.noneMatch(containsLumoGlobalImports));

// Check that imports other than lumo globals are the same
flowImports.removeAll(updater.webcomponentImports);
assertTrue(
"Flow and web-component imports must be the same, except for lumo globals",
flowImports.stream().allMatch(containsLumoGlobalImports));

}

// flow #6408
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ public static class MainView extends Component {
@JsModule("@vaadin/vaadin-lumo-styles/spacing.js")
@JsModule("@vaadin/vaadin-lumo-styles/style.js")
@JsModule("@vaadin/vaadin-lumo-styles/icons.js")
@JsModule("./lumo-includes.ts")
public static class LumoTest implements AbstractTheme {

public static final String LIGHT = "light";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ List<String> getExpectedImports() {
"@vaadin/vaadin-lumo-styles/style.js",
"@vaadin/vaadin-lumo-styles/typography.js",
"@vaadin/vaadin-lumo-styles/color.js",
"@vaadin/vaadin-lumo-styles/sizing.js",
"@vaadin/vaadin-lumo-styles/sizing.js", "./lumo-includes.ts",
"@vaadin/vaadin-date-picker/theme/lumo/vaadin-date-picker.js",
"@vaadin/vaadin-date-picker/src/vaadin-month-calendar.js",
"@vaadin/vaadin-element-mixin/vaadin-element-mixin.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@
import java.io.File;

import com.vaadin.flow.server.ExecutionFailedException;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import static com.vaadin.flow.server.Constants.TARGET;
import static com.vaadin.flow.server.frontend.FrontendUtils.FRONTEND;
import static com.vaadin.flow.server.frontend.FrontendUtils.DEFAULT_FRONTEND_DIR;
import static com.vaadin.flow.server.frontend.FrontendUtils.FRONTEND;
import static com.vaadin.flow.server.frontend.FrontendUtils.IMPORTS_WEB_COMPONENT_NAME;

public class TaskGenerateWebComponentBootstrapTest {
@Rule
Expand All @@ -42,7 +42,8 @@ public void setup() throws Exception {
frontendDirectory = temporaryFolder.newFolder(DEFAULT_FRONTEND_DIR);
File generatedFolder = temporaryFolder.newFolder(TARGET, FRONTEND);
generatedImports = new File(generatedFolder,
"flow-generated-imports.js");
IMPORTS_WEB_COMPONENT_NAME);
generatedImports.getParentFile().mkdirs();
generatedImports.createNewFile();
taskGenerateWebComponentBootstrap = new TaskGenerateWebComponentBootstrap(
frontendDirectory, generatedImports);
Expand All @@ -54,7 +55,7 @@ public void should_importGeneratedImports()
taskGenerateWebComponentBootstrap.execute();
String content = taskGenerateWebComponentBootstrap.getFileContent();
Assert.assertTrue(content.contains(
"import '../../target/frontend/flow-generated-imports.js'"));
"import '../../target/frontend/generated-flow-webcomponent-imports.js'"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ public void getModules_explcitTheme_returnAllModulesExcludingNotUsedTheme_getCla
}, false, null, false);

List<String> modules = scanner.getModules();
Assert.assertEquals(25, modules.size());
Assert.assertEquals(26, modules.size());
assertJsModules(modules);

// Theme modules should be included now
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
<!doctype html>

<head>
<script type="module"
src="web-component/themed-component.js"></script>
</head>

<body>

<span class="internal global" id="overflow">Internal should not apply</span>

<themed-component id="first"></themed-component>
<themed-component id="second"></themed-component>

</body>
<html>
<head>
<script type="module" src="web-component/themed-component.js"></script>
</head>
<body>
<h1>Lumo styles should not be applied</h1>
<div class="internal" id="internal">
Internal should not apply, this should be black
</div>
<div class="global" id="global">
Document styles should apply, this should be blue
</div>
<themed-component id="first"></themed-component>
<themed-component id="second"></themed-component>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
import java.util.stream.Collectors;

import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;

import com.vaadin.flow.component.html.testbench.DivElement;
import com.vaadin.flow.component.html.testbench.H1Element;
import com.vaadin.flow.component.html.testbench.SpanElement;
import com.vaadin.flow.testutil.ChromeBrowserTest;
import com.vaadin.testbench.TestBenchElement;
Expand Down Expand Up @@ -75,9 +75,12 @@ public void applicationTheme_GlobalCss_isUsedOnlyInEmbeddeComponent() {
Assert.assertNotEquals("Ostrich", body.getCssValue("font-family"));

Assert.assertEquals(
"Embedded style should not match external component",
"rgba(0, 0, 255, 1)",
$(SpanElement.class).id("overflow").getCssValue("color"));
"Document style should be applied outside embedded component",
"rgba(0, 0, 255, 1)", $("*").id("global").getCssValue("color"));
Assert.assertEquals(
"Theme style should not be applied outside embedded component",
"rgba(0, 0, 0, 1)", $("*").id("internal").getCssValue("color"));

getDriver().get(getRootURL() + "/themes/embedded-theme/img/bg.jpg");
Assert.assertFalse("app-theme background file should be served",
driver.getPageSource().contains("Could not navigate"));
Expand Down Expand Up @@ -225,4 +228,23 @@ public void multipleSameEmbedded_cssTargetingDocumentShouldOnlyAddElementsOneTim
2l, getCommandExecutor().executeScript(
"return window.Vaadin.theme.injectedGlobalCss.length"));
}

@Test
public void lumoImports_doNotLeakEmbeddingPage() {
open();
checkLogsForErrors();

// Ensure embedded components are loaded before testing embedding page
validateEmbeddedComponent($("themed-component").id("first"), "first");
validateEmbeddedComponent($("themed-component").id("second"), "second");

final H1Element element = $(H1Element.class).waitForFirst();
Assert.assertFalse(
"Lumo styles (typography) should not have been applied to elements in embedding page",
element.getCssValue("font-family").contains("Roboto"));
Assert.assertEquals(
"Lumo styles (colors) should not have been applied to elements in embedding page",
"rgba(0, 0, 0, 1)", element.getCssValue("color"));

}
}

0 comments on commit 1c25fcf

Please sign in to comment.