From 1c6a2f7261d500b5f03b89367122d4c1e1638ec6 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 12 May 2026 14:33:59 +0300 Subject: [PATCH 1/3] Fix CSSWatcher live reload: drop stale bindings + extract m2 designer jar Two recent CSS/localization changes regressed the simulator's live CSS reload, in different ways. 1. addThemeProps stomped user edits with stale @cn1-bind entries. PR #4884 added applyThemeBindings() inside UIManager.buildTheme so a single addThemeProps({"@accent-color": ...}) override could retune every var()-bound theme key. But CSSWatcher reloads the theme through the same code path -- and addThemeProps never clears themeConstants. When the user replaced a `var()` rule with a literal in their CSS, the recompiled theme.res no longer emitted the matching `@cn1-bind:` entry, but the previous binding was still sitting in themeConstants. applyThemeBindings happily re-overlaid the user's fresh literal value with the stale binding's resolved value, so the visible change disappeared on every reload. Fix: in buildTheme, before iterating the incoming Hashtable, detect any binding whose subject style key the new load is re-setting without re-asserting the binding alongside, and drop those bindings before the overlay pass runs. Pure `@accent-color` overrides keep working because they don't carry style keys, so no bindings are considered stale. 2. MavenUtils.findDesignerJarInM2 returned the unrunnable wrapper zip. PR #4852 added an m2 fallback for the CSSWatcher's designer-jar lookup, used whenever -Dcodename1.designer.jar isn't passed in (e.g. simulator launched from the IDE rather than `mvn cn1:run`). The helper returned `codenameone-designer--jar-with-dependencies.jar` directly from m2 -- but that artifact is a zip wrapper containing a single inner designer_1.jar (see maven/designer/pom.xml's antrun step), with no top-level Main-Class manifest. `java -jar wrapper.zip` fails with "no main manifest attribute", the CSS subprocess never starts, and the watcher silently waits for ::refresh:: lines that never come. Fix: mirror AbstractCN1Mojo.getDesignerJar's pattern -- unzip the wrapper to an `.jar-extracted/` sibling on demand and return the inner designer_1.jar so `java -jar` actually launches. Tests: - UIManagerThemeBindingsTest gains three regression cases: cssReloadDropsStaleBindingWhenRuleBecomesLiteral (the actual reproducer), cssReloadKeepsBindingWhenStillEmittedTogether (guard against an over-eager fix), and overrideOnlyReloadKeepsBindings (repeated `@accent-color` retunes still work). The first fails before the UIManager fix; all three pass after. - MavenUtilsTest is new and covers the wrapper-vs-inner-jar resolution with five cases: happy path, re-use of extracted inner jar when the wrapper hasn't changed, re-extract when the wrapper mtime advances, null when the core jar isn't in an m2 layout, and null when the designer artifact is missing. To make these actually executable, the javase pom now pins maven-surefire-plugin to 3.2.5 (the parent's 2.21.0 doesn't auto-discover JUnit Jupiter). The pre-existing CSSWatcherTest + LocationSimulationTest + JavaSEPortFontMappingTest in the same module also start running as a side effect. - pr.yml gets a new "Run JavaSE port unit tests" step so this whole test class -- which compiled but never executed -- is wired into CI. Without it, regressions in CSSWatcher/MavenUtils/JavaSEPort helpers would continue to slip through, which was the original gap the user flagged. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/pr.yml | 13 ++ .../src/com/codename1/ui/plaf/UIManager.java | 30 ++++ .../impl/javase/util/MavenUtils.java | 80 ++++++++- .../ui/plaf/UIManagerThemeBindingsTest.java | 73 ++++++++ maven/javase/pom.xml | 11 ++ .../impl/javase/util/MavenUtilsTest.java | 161 ++++++++++++++++++ 6 files changed, 364 insertions(+), 4 deletions(-) create mode 100644 maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index d6f7017c9a..5b1d408212 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -113,6 +113,19 @@ jobs: -Dcodename1.platform=javase \ -Dcn1.binaries="${CN1_BINARIES}" \ -pl codenameone-maven-plugin -am -Plocal-dev-javase test + - name: Run JavaSE port unit tests + # Surface regressions in the simulator-side helpers (CSSWatcher + # localization, MavenUtils designer-jar resolution, JavaSEPort font + # mapping). Without this step the tests in maven/javase compile but + # never execute on PRs. + working-directory: maven + env: + CN1_BINARIES: ${{ github.workspace }}/maven/target/cn1-binaries + run: | + mvn -B -Dmaven.javadoc.skip=true \ + -Dcodename1.platform=javase \ + -Dcn1.binaries="${CN1_BINARIES}" \ + -pl javase -Plocal-dev-javase test - name: Run SpotBugs for ports and Maven plugin if: ${{ matrix.java-version == 8 }} working-directory: maven diff --git a/CodenameOne/src/com/codename1/ui/plaf/UIManager.java b/CodenameOne/src/com/codename1/ui/plaf/UIManager.java index 1680a61713..91cdef3953 100644 --- a/CodenameOne/src/com/codename1/ui/plaf/UIManager.java +++ b/CodenameOne/src/com/codename1/ui/plaf/UIManager.java @@ -1790,6 +1790,31 @@ private void buildTheme(Hashtable themeProps) { Display.getInstance().installNativeTheme(); accessible = a; } + // CSSWatcher's live-reload path funnels every recompile through + // addThemeProps -> buildTheme without first clearing themeConstants. + // When the user replaces a `var()`-bound CSS rule with a literal, the + // recompiled theme.res carries the new style value but no longer emits + // the matching `@cn1-bind:` entry. The stale binding left over in + // themeConstants would otherwise let applyThemeBindings stomp the + // user's literal change with the previous binding's resolved value. + // Drop any binding whose subject key is being re-set by this load + // unless the same load also re-asserts the binding. + java.util.List bindingsToDrop = null; + Enumeration ek = themeProps.keys(); + while (ek.hasMoreElements()) { + String key = (String) ek.nextElement(); + if (key.startsWith("@")) { + continue; + } + String boundConstant = "cn1-bind:" + key; + if (themeConstants.containsKey(boundConstant) + && !themeProps.containsKey("@" + boundConstant)) { + if (bindingsToDrop == null) { + bindingsToDrop = new java.util.ArrayList(); + } + bindingsToDrop.add(boundConstant); + } + } Enumeration e = themeProps.keys(); while (e.hasMoreElements()) { String key = (String) e.nextElement(); @@ -1801,6 +1826,11 @@ private void buildTheme(Hashtable themeProps) { } this.themeProps.put(key, themeProps.get(key)); } + if (bindingsToDrop != null) { + for (String stale : bindingsToDrop) { + themeConstants.remove(stale); + } + } applyThemeBindings(); diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java b/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java index d807cca2f5..5c6b63250e 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java @@ -8,7 +8,14 @@ import com.codename1.io.Log; import com.codename1.ui.Display; import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.net.URL; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; /** * @@ -85,7 +92,20 @@ public static File findDesignerJarInM2() { if (location == null) { return null; } - File coreJar = new File(location.toURI()); + return findDesignerJarInM2(new File(location.toURI())); + } catch (Throwable t) { + // Best-effort lookup. Any unexpected layout means we can't resolve via m2. + } + return null; + } + + /** + * Test seam for {@link #findDesignerJarInM2()}: takes the codenameone-core jar + * path explicitly so the resolution logic can be exercised against a fake m2 + * layout in a temp directory. + */ + static File findDesignerJarInM2(File coreJar) { + try { // Expected layout: /com/codenameone/codenameone-core//codenameone-core-.jar File versionDir = coreJar.getParentFile(); if (versionDir == null) return null; @@ -98,9 +118,24 @@ public static File findDesignerJarInM2() { } String version = versionDir.getName(); File designerVersionDir = new File(codenameoneGroupDir, "codenameone-designer" + File.separator + version); - File designer = new File(designerVersionDir, "codenameone-designer-" + version + "-jar-with-dependencies.jar"); - if (designer.isFile()) { - return designer; + // The published jar-with-dependencies artifact is *not* directly runnable: + // maven/designer/pom.xml's antrun step renames the shaded jar to + // designer_1.jar and re-zips it, so this file is a zip wrapper containing + // a single designer_1.jar entry with no top-level Main-Class manifest. + // AbstractCN1Mojo.getDesignerJar (in the maven plugin) unzips it on demand + // and returns the inner jar; we mirror that here so the CSSWatcher + // fallback path receives a path that `java -jar` can actually launch. + File wrapperZip = new File(designerVersionDir, "codenameone-designer-" + version + "-jar-with-dependencies.jar"); + if (!wrapperZip.isFile()) { + return null; + } + File extracted = new File(wrapperZip.getParentFile(), wrapperZip.getName() + "-extracted"); + File innerJar = new File(extracted, "designer_1.jar"); + if (!innerJar.isFile() || innerJar.lastModified() < wrapperZip.lastModified()) { + extractInnerJar(wrapperZip, extracted); + } + if (innerJar.isFile()) { + return innerJar; } } catch (Throwable t) { // Best-effort lookup. Any unexpected layout means we can't resolve via m2. @@ -108,6 +143,43 @@ public static File findDesignerJarInM2() { return null; } + private static void extractInnerJar(File wrapperZip, File destDir) throws IOException { + if (!destDir.exists() && !destDir.mkdirs() && !destDir.isDirectory()) { + throw new IOException("Could not create designer extraction directory: " + destDir.getAbsolutePath()); + } + InputStream in = new FileInputStream(wrapperZip); + try { + ZipInputStream zis = new ZipInputStream(in); + try { + ZipEntry entry; + while ((entry = zis.getNextEntry()) != null) { + if (entry.isDirectory()) { + continue; + } + File out = new File(destDir, entry.getName()); + File parent = out.getParentFile(); + if (parent != null && !parent.exists() && !parent.mkdirs() && !parent.isDirectory()) { + throw new IOException("Could not create directory: " + parent.getAbsolutePath()); + } + OutputStream fos = new FileOutputStream(out); + try { + byte[] buf = new byte[8192]; + int n; + while ((n = zis.read(buf)) > 0) { + fos.write(buf, 0, n); + } + } finally { + fos.close(); + } + } + } finally { + zis.close(); + } + } finally { + in.close(); + } + } + public static boolean isRunningInJDK() { if (!isRunningInJDKChecked) { isRunningInJDKChecked = true; diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerThemeBindingsTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerThemeBindingsTest.java index 0927cc0a1a..98b8db7543 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerThemeBindingsTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerThemeBindingsTest.java @@ -126,4 +126,77 @@ public void invalidColorOverrideLeavesDefaultIntact() { b.setUIID("Button"); assertEquals(0x007aff, b.getUnselectedStyle().getFgColor()); } + + /// Simulates CSSWatcher's live-reload sequence: an initial theme load + /// (`setThemeProps`) followed by `addThemeProps` carrying a fresh + /// theme.res whose source CSS has dropped a `var()` reference in favor of + /// a literal. The reloaded Hashtable contains the new literal value for + /// `Button.fgColor` but no `@cn1-bind:Button.fgColor` entry, because the + /// CSS compiler only emits bindings for properties that still reference a + /// `var()`. The stale binding left in `themeConstants` from the first + /// load must not stomp the user's new literal value. + @Test + public void cssReloadDropsStaleBindingWhenRuleBecomesLiteral() { + Hashtable initial = new Hashtable(); + initial.put("Button.fgColor", "ff0000"); + initial.put("@cn1-bind:Button.fgColor", "accent-color"); + initial.put("@accent-color", "ff0000"); + UIManager.getInstance().setThemeProps(initial); + + Hashtable reloaded = new Hashtable(); + reloaded.put("Button.fgColor", "0000ff"); + UIManager.getInstance().addThemeProps(reloaded); + + Button b = new Button(); + b.setUIID("Button"); + assertEquals(0x0000ff, b.getUnselectedStyle().getFgColor()); + } + + /// Companion to [#cssReloadDropsStaleBindingWhenRuleBecomesLiteral]: when + /// the reload Hashtable carries BOTH the property and a fresh binding, + /// the binding still applies. This guards against an over-eager fix that + /// would drop bindings every time a style key shows up in the reload. + @Test + public void cssReloadKeepsBindingWhenStillEmittedTogether() { + Hashtable initial = new Hashtable(); + initial.put("Button.fgColor", "ff0000"); + initial.put("@cn1-bind:Button.fgColor", "accent-color"); + initial.put("@accent-color", "ff0000"); + UIManager.getInstance().setThemeProps(initial); + + Hashtable reloaded = new Hashtable(); + reloaded.put("Button.fgColor", "0000ff"); + reloaded.put("@cn1-bind:Button.fgColor", "accent-color"); + reloaded.put("@accent-color", "00ff00"); + UIManager.getInstance().addThemeProps(reloaded); + + Button b = new Button(); + b.setUIID("Button"); + assertEquals(0x00ff00, b.getUnselectedStyle().getFgColor()); + } + + /// A pure override Hashtable (no style keys, only a single `@varname` + /// constant) must not invalidate the existing bindings. This is the + /// canonical "user rebrands the accent" call path and the existing + /// [#boundThemeKeyPicksUpAccentOverride] covers a single hop; this test + /// adds a follow-up override to make sure repeated retunes keep working. + @Test + public void overrideOnlyReloadKeepsBindings() { + Hashtable initial = new Hashtable(); + initial.put("Button.fgColor", "007aff"); + initial.put("@cn1-bind:Button.fgColor", "accent-color"); + UIManager.getInstance().setThemeProps(initial); + + Hashtable firstOverride = new Hashtable(); + firstOverride.put("@accent-color", "ff2d95"); + UIManager.getInstance().addThemeProps(firstOverride); + + Hashtable secondOverride = new Hashtable(); + secondOverride.put("@accent-color", "00aa66"); + UIManager.getInstance().addThemeProps(secondOverride); + + Button b = new Button(); + b.setUIID("Button"); + assertEquals(0x00aa66, b.getUnselectedStyle().getFgColor()); + } } diff --git a/maven/javase/pom.xml b/maven/javase/pom.xml index bf65ded8c7..08437f47eb 100644 --- a/maven/javase/pom.xml +++ b/maven/javase/pom.xml @@ -125,6 +125,17 @@ + + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + maven-antrun-plugin diff --git a/maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java b/maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java new file mode 100644 index 0000000000..b8fe25c75a --- /dev/null +++ b/maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java @@ -0,0 +1,161 @@ +package com.codename1.impl.javase.util; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.File; +import java.io.FileOutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Regression coverage for {@link MavenUtils#findDesignerJarInM2(File)}. + * + *

Published {@code codenameone-designer--jar-with-dependencies.jar} artifacts + * are not directly runnable: {@code maven/designer/pom.xml} renames the shaded + * output to {@code designer_1.jar} and re-zips it, so the artifact in m2 is a + * plain zip containing a single inner jar (no top-level {@code Main-Class} + * manifest). The CSSWatcher fallback used to hand this wrapper zip to + * {@code java -jar}, which fails with "no main manifest attribute" and silently + * disables live CSS reload whenever the {@code codename1.designer.jar} system + * property isn't set (e.g. simulator launches from an IDE without going through + * {@code mvn cn1:run}).

+ */ +class MavenUtilsTest { + + @Test + void resolvesInnerDesignerJarFromWrapperZip(@TempDir Path tempDir) throws Exception { + String version = "8.0-SNAPSHOT"; + Path m2 = tempDir.resolve("m2/com/codenameone"); + Path coreDir = Files.createDirectories(m2.resolve("codenameone-core/" + version)); + Path designerDir = Files.createDirectories(m2.resolve("codenameone-designer/" + version)); + + File coreJar = coreDir.resolve("codenameone-core-" + version + ".jar").toFile(); + // The coreJar file's existence isn't required by the resolver, but write + // some bytes so a layout-aware check (e.g. "is this really a jar") would + // pass if we ever add one. + Files.write(coreJar.toPath(), new byte[]{0x50, 0x4B, 0x05, 0x06}); + + byte[] innerJarPayload = ("MANIFEST-LIKE-MARKER-" + version).getBytes("UTF-8"); + File wrapperZip = designerDir.resolve( + "codenameone-designer-" + version + "-jar-with-dependencies.jar").toFile(); + writeWrapperZip(wrapperZip, innerJarPayload); + + File resolved = MavenUtils.findDesignerJarInM2(coreJar); + + assertNotNull(resolved, "Expected resolver to find the inner designer jar"); + assertEquals("designer_1.jar", resolved.getName(), + "Resolver must return the runnable inner jar, not the wrapper zip"); + assertTrue(resolved.isFile(), "Returned path must exist as a regular file"); + assertArrayEquals(innerJarPayload, Files.readAllBytes(resolved.toPath()), + "Returned jar must be the inner jar extracted from the wrapper"); + } + + @Test + void reusesExtractedInnerJarWhenWrapperHasNotChanged(@TempDir Path tempDir) throws Exception { + String version = "8.0-SNAPSHOT"; + Path m2 = tempDir.resolve("m2/com/codenameone"); + Path coreDir = Files.createDirectories(m2.resolve("codenameone-core/" + version)); + Path designerDir = Files.createDirectories(m2.resolve("codenameone-designer/" + version)); + File coreJar = coreDir.resolve("codenameone-core-" + version + ".jar").toFile(); + Files.write(coreJar.toPath(), new byte[]{0x50, 0x4B, 0x05, 0x06}); + + byte[] payload = "payload-v1".getBytes("UTF-8"); + File wrapperZip = designerDir.resolve( + "codenameone-designer-" + version + "-jar-with-dependencies.jar").toFile(); + writeWrapperZip(wrapperZip, payload); + + File firstResolve = MavenUtils.findDesignerJarInM2(coreJar); + assertNotNull(firstResolve); + long extractedMtime = firstResolve.lastModified(); + + // Make sure mtime comparison won't accidentally re-extract because of + // FS resolution. A second resolve with no wrapper change should be a no-op. + File secondResolve = MavenUtils.findDesignerJarInM2(coreJar); + assertNotNull(secondResolve); + assertEquals(firstResolve.getAbsolutePath(), secondResolve.getAbsolutePath()); + assertEquals(extractedMtime, secondResolve.lastModified(), + "Inner jar should not be re-extracted when wrapper hasn't changed"); + } + + @Test + void reExtractsWhenWrapperZipIsNewerThanExtractedJar(@TempDir Path tempDir) throws Exception { + String version = "8.0-SNAPSHOT"; + Path m2 = tempDir.resolve("m2/com/codenameone"); + Path coreDir = Files.createDirectories(m2.resolve("codenameone-core/" + version)); + Path designerDir = Files.createDirectories(m2.resolve("codenameone-designer/" + version)); + File coreJar = coreDir.resolve("codenameone-core-" + version + ".jar").toFile(); + Files.write(coreJar.toPath(), new byte[]{0x50, 0x4B, 0x05, 0x06}); + File wrapperZip = designerDir.resolve( + "codenameone-designer-" + version + "-jar-with-dependencies.jar").toFile(); + + writeWrapperZip(wrapperZip, "payload-v1".getBytes("UTF-8")); + File first = MavenUtils.findDesignerJarInM2(coreJar); + assertNotNull(first); + assertArrayEquals("payload-v1".getBytes("UTF-8"), Files.readAllBytes(first.toPath())); + + // Rewrite the wrapper with a new payload and bump its mtime past the + // extracted inner jar. The resolver must notice and re-extract. + writeWrapperZip(wrapperZip, "payload-v2".getBytes("UTF-8")); + long bumped = first.lastModified() + 5_000L; + assertTrue(wrapperZip.setLastModified(bumped), + "FS must support setLastModified for this test"); + + File second = MavenUtils.findDesignerJarInM2(coreJar); + assertNotNull(second); + assertArrayEquals("payload-v2".getBytes("UTF-8"), Files.readAllBytes(second.toPath())); + } + + @Test + void returnsNullForUnrelatedJarLocation(@TempDir Path tempDir) throws Exception { + // Core jar living outside an m2 layout: resolver must give up rather + // than return a phantom path. CSSWatcher then falls through to its + // ~/.codenameone/designer_1.jar legacy fallback. + File notInM2 = tempDir.resolve("build/codenameone-core.jar").toFile(); + Files.createDirectories(notInM2.getParentFile().toPath()); + Files.write(notInM2.toPath(), new byte[]{0x50, 0x4B, 0x05, 0x06}); + + assertNull(MavenUtils.findDesignerJarInM2(notInM2)); + } + + @Test + void returnsNullWhenDesignerArtifactMissing(@TempDir Path tempDir) throws Exception { + // Core jar lives in a valid m2 layout but the designer artifact has not + // been resolved into the local repo (e.g. cn1lib project that doesn't + // run the maven plugin). Resolver should report null, not throw. + String version = "8.0-SNAPSHOT"; + Path coreDir = Files.createDirectories( + tempDir.resolve("m2/com/codenameone/codenameone-core/" + version)); + File coreJar = coreDir.resolve("codenameone-core-" + version + ".jar").toFile(); + Files.write(coreJar.toPath(), new byte[]{0x50, 0x4B, 0x05, 0x06}); + + assertNull(MavenUtils.findDesignerJarInM2(coreJar)); + } + + private static void writeWrapperZip(File wrapperZip, byte[] innerJarPayload) throws Exception { + // Mirror the layout produced by maven/designer/pom.xml's antrun step: + // a plain zip whose sole entry is a designer_1.jar file. No manifest, + // not directly runnable. + FileOutputStream fos = new FileOutputStream(wrapperZip); + try { + ZipOutputStream zos = new ZipOutputStream(fos); + try { + zos.putNextEntry(new ZipEntry("designer_1.jar")); + zos.write(innerJarPayload); + zos.closeEntry(); + } finally { + zos.close(); + } + } finally { + fos.close(); + } + } +} From a4e9b3a9c1bb8fff5d0a0ad206b85ece76ff1017 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 12 May 2026 15:05:16 +0300 Subject: [PATCH 2/3] Address PR review: harden Zip Slip + install javase deps in CI step - MavenUtils.extractInnerJar no longer derives a File path from ZipEntry.getName(). CodeQL flagged the previous loop as a Zip Slip risk because a wrapper containing `../../etc/passwd` would have been written outside the extraction directory. The wrapper produced by maven/designer/pom.xml has a single designer_1.jar entry by design, so the extractor now (a) writes only to a single fixed destination path under destDir and (b) only matches entries whose literal name equals "designer_1.jar". Anything else is skipped; if the canonical entry is absent, the method throws. Two new MavenUtilsTest cases: refusesPathTraversalEntriesAndDoesNotWriteOutsideExtractDir packs a `../../escaped.txt` entry and asserts no escaped file appears in the temp root; skipsUnexpectedEntriesAndStillExtractsDesignerJar mixes a README and a subdir/other.jar with the real designer_1.jar and asserts only the inner jar lands on disk. - pr.yml's new "Run JavaSE port unit tests" step failed with "Could not find artifact com.codenameone:sqlite-jdbc:jar:8.0-SNAPSHOT" on all three matrix entries (Java 8/17/21). The earlier "Build Codename One" step builds core-unittests with -am, which doesn't install sqlite-jdbc into the local repo. Split the new step into two mvn invocations: first install javase's transitive deps without running their tests, then run javase's tests in isolation. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/pr.yml | 9 +- .../impl/javase/util/MavenUtils.java | 28 ++++-- .../impl/javase/util/MavenUtilsTest.java | 89 +++++++++++++++++++ 3 files changed, 120 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 5b1d408212..edf80b97aa 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -117,11 +117,18 @@ jobs: # Surface regressions in the simulator-side helpers (CSSWatcher # localization, MavenUtils designer-jar resolution, JavaSEPort font # mapping). Without this step the tests in maven/javase compile but - # never execute on PRs. + # never execute on PRs. We install the javase module's transitive + # deps first (sqlite-jdbc, factory, etc. -- earlier steps don't + # publish them to the local repo) so the test invocation can resolve + # them, then run javase's tests in isolation. working-directory: maven env: CN1_BINARIES: ${{ github.workspace }}/maven/target/cn1-binaries run: | + mvn -B -Dmaven.javadoc.skip=true \ + -Dcodename1.platform=javase \ + -Dcn1.binaries="${CN1_BINARIES}" \ + -pl javase -am -Plocal-dev-javase -DskipTests install mvn -B -Dmaven.javadoc.skip=true \ -Dcodename1.platform=javase \ -Dcn1.binaries="${CN1_BINARIES}" \ diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java b/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java index 5c6b63250e..4bd344f9a6 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java @@ -143,10 +143,25 @@ static File findDesignerJarInM2(File coreJar) { return null; } + private static final String INNER_JAR_NAME = "designer_1.jar"; + + /** + * Extracts the single expected inner jar from the designer wrapper artifact. + * + *

The wrapper produced by {@code maven/designer/pom.xml} contains exactly + * one entry named {@code designer_1.jar} at the root. To stay safe against + * Zip Slip even if an unexpected artifact is dropped in m2, this method: + * (1) writes only to a single, fixed destination path under {@code destDir} + * (never derived from the archive's entry name), and (2) skips any entry + * whose name isn't the literal expected filename. A malicious entry like + * {@code ../../etc/passwd} therefore never participates in path + * construction; in the worst case the loop finds no match and throws.

+ */ private static void extractInnerJar(File wrapperZip, File destDir) throws IOException { if (!destDir.exists() && !destDir.mkdirs() && !destDir.isDirectory()) { throw new IOException("Could not create designer extraction directory: " + destDir.getAbsolutePath()); } + File innerJar = new File(destDir, INNER_JAR_NAME); InputStream in = new FileInputStream(wrapperZip); try { ZipInputStream zis = new ZipInputStream(in); @@ -156,12 +171,12 @@ private static void extractInnerJar(File wrapperZip, File destDir) throws IOExce if (entry.isDirectory()) { continue; } - File out = new File(destDir, entry.getName()); - File parent = out.getParentFile(); - if (parent != null && !parent.exists() && !parent.mkdirs() && !parent.isDirectory()) { - throw new IOException("Could not create directory: " + parent.getAbsolutePath()); + if (!INNER_JAR_NAME.equals(entry.getName())) { + // Unexpected entry. Skip it rather than materialize a + // file path derived from untrusted archive metadata. + continue; } - OutputStream fos = new FileOutputStream(out); + OutputStream fos = new FileOutputStream(innerJar); try { byte[] buf = new byte[8192]; int n; @@ -171,7 +186,10 @@ private static void extractInnerJar(File wrapperZip, File destDir) throws IOExce } finally { fos.close(); } + return; } + throw new IOException("Wrapper zip does not contain a " + INNER_JAR_NAME + + " entry: " + wrapperZip.getAbsolutePath()); } finally { zis.close(); } diff --git a/maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java b/maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java index b8fe25c75a..c63da74916 100644 --- a/maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java +++ b/maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java @@ -12,6 +12,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -126,6 +127,94 @@ void returnsNullForUnrelatedJarLocation(@TempDir Path tempDir) throws Exception assertNull(MavenUtils.findDesignerJarInM2(notInM2)); } + @Test + void refusesPathTraversalEntriesAndDoesNotWriteOutsideExtractDir(@TempDir Path tempDir) throws Exception { + // CodeQL flagged the original extraction loop as a Zip Slip risk: it + // built `new File(destDir, entry.getName())` from untrusted archive + // metadata. Pack a wrapper whose only entry is a `../../etc/passwd`- + // style traversal name and verify the resolver refuses to extract + // (no file outside the extracted dir, no inner jar produced). + String version = "8.0-SNAPSHOT"; + Path m2 = tempDir.resolve("m2/com/codenameone"); + Path coreDir = Files.createDirectories(m2.resolve("codenameone-core/" + version)); + Path designerDir = Files.createDirectories(m2.resolve("codenameone-designer/" + version)); + File coreJar = coreDir.resolve("codenameone-core-" + version + ".jar").toFile(); + Files.write(coreJar.toPath(), new byte[]{0x50, 0x4B, 0x05, 0x06}); + + File wrapperZip = designerDir.resolve( + "codenameone-designer-" + version + "-jar-with-dependencies.jar").toFile(); + File traversalSentinel = tempDir.resolve("escaped.txt").toFile(); + FileOutputStream fos = new FileOutputStream(wrapperZip); + try { + ZipOutputStream zos = new ZipOutputStream(fos); + try { + // The relative `../../escaped.txt` resolves to tempDir/escaped.txt + // if the extractor were vulnerable: extractedDir lives at + // designerDir/-extracted/, so two ".." pops back to + // tempDir's root. + zos.putNextEntry(new ZipEntry("../../escaped.txt")); + zos.write("if-you-see-me-zip-slip-happened".getBytes("UTF-8")); + zos.closeEntry(); + } finally { + zos.close(); + } + } finally { + fos.close(); + } + + File resolved = MavenUtils.findDesignerJarInM2(coreJar); + assertNull(resolved, "Resolver must report failure when the wrapper has no designer_1.jar"); + assertFalse(traversalSentinel.exists(), + "Traversal entry must not be written outside the extraction directory"); + } + + @Test + void skipsUnexpectedEntriesAndStillExtractsDesignerJar(@TempDir Path tempDir) throws Exception { + // Hardening guard: even if a future wrapper variant adds extra files + // alongside designer_1.jar, the extractor should ignore them and only + // surface the canonical inner jar. + String version = "8.0-SNAPSHOT"; + Path m2 = tempDir.resolve("m2/com/codenameone"); + Path coreDir = Files.createDirectories(m2.resolve("codenameone-core/" + version)); + Path designerDir = Files.createDirectories(m2.resolve("codenameone-designer/" + version)); + File coreJar = coreDir.resolve("codenameone-core-" + version + ".jar").toFile(); + Files.write(coreJar.toPath(), new byte[]{0x50, 0x4B, 0x05, 0x06}); + + File wrapperZip = designerDir.resolve( + "codenameone-designer-" + version + "-jar-with-dependencies.jar").toFile(); + byte[] expectedPayload = "real-designer-bytes".getBytes("UTF-8"); + FileOutputStream fos = new FileOutputStream(wrapperZip); + try { + ZipOutputStream zos = new ZipOutputStream(fos); + try { + zos.putNextEntry(new ZipEntry("README.txt")); + zos.write("noise".getBytes("UTF-8")); + zos.closeEntry(); + zos.putNextEntry(new ZipEntry("designer_1.jar")); + zos.write(expectedPayload); + zos.closeEntry(); + zos.putNextEntry(new ZipEntry("subdir/other.jar")); + zos.write("nope".getBytes("UTF-8")); + zos.closeEntry(); + } finally { + zos.close(); + } + } finally { + fos.close(); + } + + File resolved = MavenUtils.findDesignerJarInM2(coreJar); + assertNotNull(resolved); + assertEquals("designer_1.jar", resolved.getName()); + assertArrayEquals(expectedPayload, Files.readAllBytes(resolved.toPath())); + // The extra entries must not have been written to the extraction dir. + File extractedDir = resolved.getParentFile(); + assertFalse(new File(extractedDir, "README.txt").exists(), + "Unexpected entries must be skipped, not written"); + assertFalse(new File(extractedDir, "subdir").exists(), + "Unexpected nested entries must be skipped, not written"); + } + @Test void returnsNullWhenDesignerArtifactMissing(@TempDir Path tempDir) throws Exception { // Core jar lives in a valid m2 layout but the designer artifact has not From 318716bcdfb5ddc4c680bd53f4ca604714ad321e Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 12 May 2026 16:56:48 +0300 Subject: [PATCH 3/3] UIManager: scope stale-binding drop to addThemeProps, fix iOS hang The previous fix ran the stale-binding preprocessing inside buildTheme, which is also called from the @includeNativeBool layered initial load (setThemePropsImpl -> buildTheme -> Display.installNativeTheme() -> buildTheme(native) -> outer buildTheme(userTheme) continues). After the native theme installs its bindings into themeConstants, the outer call's preprocessing would drop them whenever the user's app theme.css set a literal value for the same UIID -- which the existing iOS / Android screenshot goldens were captured against. The iOS PR check hit this: the device-runner log shows the suite ran fine through ChartCubicLineScreenshotTest and then hung in ChartBarScreenshotTest setup until the 30-minute timeout fired. The inconsistent themeConstants state left over once the layered native bindings were dropped manifests as a hang in chart-component initialization (presumably a Style.derive cycle or similar) rather than as a pixel diff. Move the drop pre-pass out of buildTheme and into a new dropSupersededBindings() called only from addThemeProps. This keeps the CSSWatcher reload fix (the actual reported regression) and the companion regression tests passing, while restoring the original behavior of the layered initial-load path -- bindings declared by the native theme via @includeNativeBool stay live, user-app literals don't silently strip them out. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/com/codename1/ui/plaf/UIManager.java | 69 +++++++++++-------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/plaf/UIManager.java b/CodenameOne/src/com/codename1/ui/plaf/UIManager.java index 91cdef3953..dca3c4cd2f 100644 --- a/CodenameOne/src/com/codename1/ui/plaf/UIManager.java +++ b/CodenameOne/src/com/codename1/ui/plaf/UIManager.java @@ -1500,6 +1500,7 @@ private void resetThemeProps(Hashtable installedTheme) { /// - `themeProps`: the properties of the given theme public void addThemeProps(Hashtable themeProps) { if (accessible) { + dropSupersededBindings(themeProps); buildTheme(themeProps); styles.clear(); selectedStyles.clear(); @@ -1508,6 +1509,44 @@ public void addThemeProps(Hashtable themeProps) { } } + /// CSSWatcher's live-reload funnels every recompile through + /// [#addThemeProps], which never clears [#themeConstants]. When a user + /// replaces a `var()`-bound CSS rule with a literal, the recompiled + /// theme.res carries the new style value but no longer emits the + /// matching `@cn1-bind:` entry. Without intervention the stale + /// binding left in `themeConstants` would let [#applyThemeBindings] + /// stomp the user's literal change back to the previous binding's + /// resolved value -- visibly hiding every CSS edit. + /// + /// This pre-pass runs only on the overlay entry point (`addThemeProps`), + /// not on the full reset path ([#setThemePropsImpl] -> [#buildTheme], + /// which clears `themeConstants` itself, and the `@includeNativeBool` + /// layered initial load whose existing screenshots depend on bindings + /// staying in place). For each style key being re-set by the incoming + /// load that does NOT re-assert its binding, drop the matching binding + /// from `themeConstants` so the new literal wins. + private void dropSupersededBindings(Hashtable themeProps) { + if (themeProps == null || themeConstants == null || themeConstants.isEmpty()) { + return; + } + Enumeration e = themeProps.keys(); + while (e.hasMoreElements()) { + Object keyObj = e.nextElement(); + if (!(keyObj instanceof String)) { + continue; + } + String key = (String) keyObj; + if (key.startsWith("@")) { + continue; + } + String boundConstant = "cn1-bind:" + key; + if (themeConstants.containsKey(boundConstant) + && !themeProps.containsKey("@" + boundConstant)) { + themeConstants.remove(boundConstant); + } + } + } + /// Scales the font sizes of the current theme by the given factor, e.g. /// a factor of 1.2 increases all font sizes by 20% and a factor of 0.8 /// decreases them by 20%. Only fonts that support scaling (TTF or native: @@ -1790,31 +1829,6 @@ private void buildTheme(Hashtable themeProps) { Display.getInstance().installNativeTheme(); accessible = a; } - // CSSWatcher's live-reload path funnels every recompile through - // addThemeProps -> buildTheme without first clearing themeConstants. - // When the user replaces a `var()`-bound CSS rule with a literal, the - // recompiled theme.res carries the new style value but no longer emits - // the matching `@cn1-bind:` entry. The stale binding left over in - // themeConstants would otherwise let applyThemeBindings stomp the - // user's literal change with the previous binding's resolved value. - // Drop any binding whose subject key is being re-set by this load - // unless the same load also re-asserts the binding. - java.util.List bindingsToDrop = null; - Enumeration ek = themeProps.keys(); - while (ek.hasMoreElements()) { - String key = (String) ek.nextElement(); - if (key.startsWith("@")) { - continue; - } - String boundConstant = "cn1-bind:" + key; - if (themeConstants.containsKey(boundConstant) - && !themeProps.containsKey("@" + boundConstant)) { - if (bindingsToDrop == null) { - bindingsToDrop = new java.util.ArrayList(); - } - bindingsToDrop.add(boundConstant); - } - } Enumeration e = themeProps.keys(); while (e.hasMoreElements()) { String key = (String) e.nextElement(); @@ -1826,11 +1840,6 @@ private void buildTheme(Hashtable themeProps) { } this.themeProps.put(key, themeProps.get(key)); } - if (bindingsToDrop != null) { - for (String stale : bindingsToDrop) { - themeConstants.remove(stale); - } - } applyThemeBindings();