You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by ad...@apache.org on 2026/05/09 15:46:48 UTC

(wicket) branch wicket-9.x updated: Wicket 9.x security fixes (#1450)

This is an automated email from the ASF dual-hosted git repository.

bitstorm pushed a commit to branch wicket-9.x
in repository https://gitbox.apache.org/repos/asf/wicket.git


The following commit(s) were added to refs/heads/wicket-9.x by this push:
     new 7b999bc372 Wicket 9.x security fixes (#1450)
7b999bc372 is described below

commit 7b999bc372c6f894e9a69ca8c92c62f5ca7acd42
Author: pedrosans <pe...@apache.org>
AuthorDate: Sat May 9 12:46:41 2026 -0300

    Wicket 9.x security fixes (#1450)
    
    * Sanitize file upload data (#1432)
    
    * Security Vulnerabilty - Path Traversal Fix
    
    * Code review comments implemented
    
    * Added JS sanitazing and and fix for PackageResource
    
    ---------
    
    Co-authored-by: Ravishanker Kusuma <ra...@hayageek.com>
    Co-authored-by: Andrea Del Bene <ad...@apache.org>
---
 .../wicket/core/util/string/JavaScriptUtils.java   |  20 +++
 .../upload/resource/FolderUploadsFileManager.java  |  66 ++++++++-
 .../wicket/markup/html/link/ExternalLink.java      |   8 +-
 .../org/apache/wicket/markup/html/link/Link.java   |   2 +-
 .../wicket/markup/html/link/PopupSettings.java     |   7 +-
 .../wicket/request/resource/PackageResource.java   |  54 +++----
 .../core/util/string/JavaScriptUtilsTest.java      |   7 +
 .../wicket/markup/html/PackageResourceTest.java    |  70 ++++++++-
 .../resource/FolderUploadsFileManagerTest.java     |  96 +++++++++++++
 .../markup/html/link/ClientSideImageMapTest.java   |  76 +++++-----
 .../wicket/markup/html/link/ExternalLinkTest.java  | 159 ++++++++++++++-------
 .../apache/wicket/markup/html/link/LinkTest.java   | 106 ++++++++++++++
 .../snake_case/TestPageInsideSnakeCasePackage.html |  11 ++
 .../TestPageInsideSnakeCasePackage.java}           |  61 +++-----
 .../apache/wicket/markup/html/snake_case/style.css |   0
 .../autocomplete/AbstractAutoCompleteRenderer.java |   3 +-
 .../AbstractAutoCompleteRendererTest.java          |  65 +++++++++
 17 files changed, 631 insertions(+), 180 deletions(-)

diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java b/wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java
index 4038fe53d2..e487a9a245 100644
--- a/wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java
+++ b/wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java
@@ -104,6 +104,26 @@ public class JavaScriptUtils
 		return s;
 	}
 
+	/**
+	 * Escape single and double quotes so that they can be part of e.g. an alert call.
+	 *
+	 * Note: JSON values need to escape only the double quote, so this method wont help.
+	 *
+	 * @param input
+	 *            the JavaScript which needs to be escaped
+	 * @return Escaped version of the input
+	 */
+	public static CharSequence escapeQuotesAndBackslash(final CharSequence input)
+	{
+		CharSequence s = input;
+		if (s != null)
+		{
+			s = Strings.replaceAll(s, "\\", "\\\\");
+			s = escapeQuotes(s);
+		}
+		return s;
+	}
+
 	/**
 	 * Write a reference to a javascript file to the response object
 	 * 
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManager.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManager.java
index 0146b82099..f877acc9a6 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManager.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManager.java
@@ -18,7 +18,10 @@ package org.apache.wicket.markup.html.form.upload.resource;
 
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
 import org.apache.wicket.WicketRuntimeException;
 import org.apache.wicket.markup.html.form.upload.FileUpload;
 import org.apache.wicket.util.file.File;
@@ -60,14 +63,62 @@ public class FolderUploadsFileManager implements IUploadsFileManager
         return folder;
     }
 
+    /**
+     * Returns the canonical path for a directory for use in path traversal checks.
+     * Uses {@code toRealPath()} when the directory exists; falls back to
+     * {@code toAbsolutePath().normalize()} only when the directory does not yet exist
+     * (e.g. an upload sub-folder that will be created during {@link #save}).
+     * Other {@link IOException} subtypes (e.g. permission errors) are intentionally
+     * re-thrown so they are not silently swallowed.
+     */
+    private static Path getPathForComparison(java.io.File dir) throws IOException
+    {
+        try
+        {
+            return dir.toPath().toRealPath();
+        }
+        catch (NoSuchFileException e)
+        {
+            return dir.toPath().toAbsolutePath().normalize();
+        }
+    }
+
+    /**
+     * Validates {@code uploadFieldId} and {@code clientFileName} against the base folder to
+     * prevent path traversal, and returns the fully resolved, canonical target file.
+     *
+     * @throws SecurityException if either component would escape the base folder
+     */
+    private java.io.File resolveTargetFile(String uploadFieldId, String clientFileName)
+            throws IOException
+    {
+        Path baseFolderPath = getFolder().toPath().toRealPath();
+        java.io.File uploadFieldFolder = new File(getFolder(), uploadFieldId).getCanonicalFile();
+        if (!uploadFieldFolder.toPath().startsWith(baseFolderPath))
+        {
+            throw new SecurityException("Path traversal detected in uploadFieldId");
+        }
+        java.io.File target = new File(uploadFieldFolder, clientFileName).getCanonicalFile();
+        Path uploadFieldFolderPath = getPathForComparison(uploadFieldFolder);
+        if (!target.toPath().startsWith(uploadFieldFolderPath))
+        {
+            throw new SecurityException("Path traversal detected in client filename");
+        }
+        return target;
+    }
+
     @Override
     public void save(FileUpload fileItem, String uploadFieldId)
     {
-        File uploadFieldFolder = new File(getFolder(), uploadFieldId);
-        uploadFieldFolder.mkdirs();
         try
         {
-            IOUtils.copy(fileItem.getInputStream(),  new FileOutputStream(new File(uploadFieldFolder, fileItem.getClientFileName())));
+            java.io.File target = resolveTargetFile(uploadFieldId, fileItem.getClientFileName());
+            Files.createDirectories(target.toPath().getParent());
+            try (InputStream in = fileItem.getInputStream();
+                 FileOutputStream out = new FileOutputStream(target))
+            {
+                IOUtils.copy(in, out);
+            }
         }
         catch (IOException e)
         {
@@ -78,6 +129,13 @@ public class FolderUploadsFileManager implements IUploadsFileManager
     @Override
     public File getFile(String uploadFieldId, String clientFileName)
     {
-        return new File(new File(getFolder(), uploadFieldId), clientFileName);
+        try
+        {
+            return new File(resolveTargetFile(uploadFieldId, clientFileName));
+        }
+        catch (IOException e)
+        {
+            throw new WicketRuntimeException(e);
+        }
     }
 }
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java
index 3217ee37d9..09607c8fc8 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java
@@ -16,6 +16,7 @@
  */
 package org.apache.wicket.markup.html.link;
 
+import org.apache.wicket.core.util.string.JavaScriptUtils;
 import org.apache.wicket.markup.ComponentTag;
 import org.apache.wicket.markup.head.IHeaderResponse;
 import org.apache.wicket.markup.head.OnEventHeaderItem;
@@ -188,7 +189,7 @@ public class ExternalLink extends AbstractLink
 		{
 			if (popupSettings != null)
 			{
-				popupSettings.setTarget("'" + url + "'");
+				popupSettings.setTarget(url);
 				response.render(OnEventHeaderItem.forComponent(this, "click",
 					popupSettings.getPopupJavaScript()));
 				return;
@@ -205,8 +206,9 @@ public class ExternalLink extends AbstractLink
 				// in firefox when the element is quickly clicked 3 times a second request is
 				// generated during page load. This check ensures that the click is ignored
 				response.render(OnEventHeaderItem.forComponent(this, "click",
-					"var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; "
-						+ "if (win == window) { window.location.href='" + url
+					"var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; " //
+						+ "if (win == window) { window.location.href='" //
+						+ JavaScriptUtils.escapeQuotes(url) //
 						+ "'; } ;return false"));
 				return;
 			}
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java
index a374db164f..872ef36147 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java
@@ -407,7 +407,7 @@ public abstract class Link<T> extends AbstractLink implements IRequestListener,
 			// next check for popup settings
 			if (popupSettings != null)
 			{
-				popupSettings.setTarget("'" + url + "'");
+				popupSettings.setTarget(url.toString());
 				response.render(OnEventHeaderItem.forComponent(this, "click",
 					popupSettings.getPopupJavaScript()));
 				return;
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/PopupSettings.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/PopupSettings.java
index 82dc08f82f..e62b3e01be 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/PopupSettings.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/PopupSettings.java
@@ -16,6 +16,7 @@
  */
 package org.apache.wicket.markup.html.link;
 
+import org.apache.wicket.core.util.string.JavaScriptUtils;
 import org.apache.wicket.util.io.IClusterable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -155,8 +156,10 @@ public class PopupSettings implements IClusterable
 			windowTitle = windowTitle.replaceAll("\\W", "_");
 		}
 
-	    StringBuilder script = new StringBuilder("var w = window.open(" + target + ", '").append(
-			windowTitle).append("', '");
+		StringBuilder script = new StringBuilder(//
+			"var w = window.open('"//
+				+ JavaScriptUtils.escapeQuotes(target) //
+				+ "', '").append(windowTitle).append("', '");
 
 		script.append("scrollbars=").append(flagToString(SCROLLBARS));
 		script.append(",location=").append(flagToString(LOCATION_BAR));
diff --git a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java
index 0cbc7b84c7..c54fd52c32 100644
--- a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java
+++ b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java
@@ -16,16 +16,6 @@
  */
 package org.apache.wicket.request.resource;
 
-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.Serializable;
-import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
-import java.time.Instant;
-import java.util.Locale;
-import java.util.Objects;
-import javax.servlet.http.HttpServletResponse;
 import org.apache.wicket.Application;
 import org.apache.wicket.IWicketInternalException;
 import org.apache.wicket.Session;
@@ -45,7 +35,6 @@ import org.apache.wicket.response.StringResponse;
 import org.apache.wicket.util.io.IOUtils;
 import org.apache.wicket.util.lang.Classes;
 import org.apache.wicket.util.lang.Packages;
-import org.apache.wicket.util.resource.IFixedLocationResourceStream;
 import org.apache.wicket.util.resource.IResourceStream;
 import org.apache.wicket.util.resource.ResourceStreamNotFoundException;
 import org.apache.wicket.util.resource.ResourceStreamWrapper;
@@ -53,6 +42,17 @@ import org.apache.wicket.util.string.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.servlet.http.HttpServletResponse;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Serializable;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.time.Instant;
+import java.util.Locale;
+import java.util.Objects;
+
 /**
  * Represents a localizable static resource.
  * <p>
@@ -555,38 +555,18 @@ public class PackageResource extends AbstractResource implements IStaticCacheabl
 
 	private IResourceStream internalGetResourceStream(final String style, final Locale locale)
 	{
+		if (!accept(absolutePath))
+		{
+			throw new PackageResourceBlockedException(
+				"Access denied to (static) package resource " + absolutePath + ". See IPackageResourceGuard");
+		}
+
 		IResourceStreamLocator resourceStreamLocator = Application.get()
 			.getResourceSettings()
 			.getResourceStreamLocator();
 		IResourceStream resourceStream = resourceStreamLocator.locate(getScope(), absolutePath,
 			style, variation, locale, null, false);
 
-		String realPath = absolutePath;
-		if (resourceStream instanceof IFixedLocationResourceStream)
-		{
-			realPath = ((IFixedLocationResourceStream)resourceStream).locationAsString();
-			if (realPath != null)
-			{
-				int index = realPath.indexOf(absolutePath);
-				if (index != -1)
-				{
-					realPath = realPath.substring(index);
-				}
-			}
-			else
-			{
-				realPath = absolutePath;
-			}
-
-		}
-
-		if (accept(realPath) == false)
-		{
-			throw new PackageResourceBlockedException(
-				"Access denied to (static) package resource " + absolutePath +
-					". See IPackageResourceGuard");
-		}
-
 		if (resourceStream != null)
 		{
 			resourceStream = new ProcessingResourceStream(resourceStream);
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/util/string/JavaScriptUtilsTest.java b/wicket-core/src/test/java/org/apache/wicket/core/util/string/JavaScriptUtilsTest.java
index b849101615..07b5c4ec10 100644
--- a/wicket-core/src/test/java/org/apache/wicket/core/util/string/JavaScriptUtilsTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/core/util/string/JavaScriptUtilsTest.java
@@ -16,10 +16,12 @@
  */
 package org.apache.wicket.core.util.string;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import org.apache.wicket.response.StringResponse;
 import org.apache.wicket.util.value.AttributeMap;
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -97,4 +99,9 @@ class JavaScriptUtilsTest
 			JavaScriptUtils.SCRIPT_OPEN_TAG);
 		assertEquals("\n/*]]>*/\n</script>\n", JavaScriptUtils.SCRIPT_CLOSE_TAG);
 	}
+
+	@Test
+	void escapeQuotesAndBackslash(){
+		assertThat(JavaScriptUtils.escapeQuotesAndBackslash("alert('foo\\tbar')")).isEqualTo("alert(\\'foo\\\\tbar\\')");
+	}
 }
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/PackageResourceTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/PackageResourceTest.java
index 11bd855db2..0a0c00f4f0 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/html/PackageResourceTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/PackageResourceTest.java
@@ -16,16 +16,11 @@
  */
 package org.apache.wicket.markup.html;
 
-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.assertTrue;
-
-import java.util.Locale;
-
 import org.apache.wicket.Application;
 import org.apache.wicket.SharedResources;
+import org.apache.wicket.markup.html.snake_case.TestPageInsideSnakeCasePackage;
 import org.apache.wicket.protocol.http.WebApplication;
+import org.apache.wicket.protocol.https.HttpPage;
 import org.apache.wicket.request.resource.JavaScriptPackageResource;
 import org.apache.wicket.request.resource.PackageResource;
 import org.apache.wicket.request.resource.PackageResourceReference;
@@ -35,6 +30,12 @@ import org.apache.wicket.util.tester.WicketTestCase;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import java.util.Locale;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.*;
+
 /**
  * Tests for package resources.
  * 
@@ -193,4 +194,59 @@ public class PackageResourceTest extends WicketTestCase
 		final String contentType = tester.getLastResponse().getContentType();
 		assertEquals("text/javascript; charset=" + encoding, contentType);
 	}
+
+	@Test
+	void getResourceStream()
+	{
+		PackageResource resource = new PackageResourceReference(PackageResourceTest.class,
+			"packaged1.txt").getResource();
+		assertThat(resource.getResourceStream()).isNotNull();
+	}
+
+	@Test
+	void dontGetResourceStream()
+	{
+		PackageResource resource = new PackageResourceReference(HttpPage.class,
+			"HttpPage.html").getResource();
+		assertThatThrownBy(resource::getResourceStream).isInstanceOf(
+			PackageResource.PackageResourceBlockedException.class);
+	}
+
+	@Test
+	void dontGetResourceStreamIfNameHasSuffix()
+	{
+		PackageResource resource = new PackageResourceReference(HttpPage.class,
+			"HttpPage_en.html").getResource();
+		assertThatThrownBy(resource::getResourceStream).isInstanceOf(
+			PackageResource.PackageResourceBlockedException.class);
+	}
+
+	@Test
+	void getResourceStreamInSnakeCasePackage()
+	{
+		PackageResource resource = new PackageResourceReference(
+			TestPageInsideSnakeCasePackage.class, "style.css").getResource();
+		assertThat(resource.getResourceStream()).isNotNull();
+	}
+
+	@Test
+	void dontGetResourceStreamInSnakeCasePackage()
+	{
+		PackageResource resource = new PackageResourceReference(
+			TestPageInsideSnakeCasePackage.class,
+			"TestPageInsideSnakeCasePackage.html").getResource();
+		assertThatThrownBy(resource::getResourceStream).isInstanceOf(
+			PackageResource.PackageResourceBlockedException.class);
+	}
+
+	@Test
+	void dontGetResourceStreamInSnakeCasePackageIfNameHasSuffix()
+	{
+		PackageResource resource = new PackageResourceReference(
+			TestPageInsideSnakeCasePackage.class,
+			"TestPageInsideSnakeCasePackage_en.html").getResource();
+		assertThatThrownBy(resource::getResourceStream).isInstanceOf(
+			PackageResource.PackageResourceBlockedException.class);
+	}
+
 }
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManagerTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManagerTest.java
new file mode 100644
index 0000000000..4930aed2d7
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManagerTest.java
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.markup.html.form.upload.resource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.apache.wicket.markup.html.form.upload.FileUpload;
+import org.apache.wicket.util.file.File;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+class FolderUploadsFileManagerTest
+{
+	@TempDir
+	Path tempDir;
+
+	@Test
+	void getFileRejectsTraversalInUploadFieldId()
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+
+		assertThrows(SecurityException.class, () -> manager.getFile("../escaped", "safe.txt"));
+	}
+
+	@Test
+	void getFileRejectsTraversalInClientFileName()
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+
+		assertThrows(SecurityException.class, () -> manager.getFile("uploadField", "../../etc/passwd"));
+	}
+
+	@Test
+	void saveRejectsTraversalInUploadFieldId() throws IOException
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+		FileUpload fileUpload = mock(FileUpload.class);
+		when(fileUpload.getClientFileName()).thenReturn("safe.txt");
+		when(fileUpload.getInputStream())
+			.thenReturn(new ByteArrayInputStream("content".getBytes(StandardCharsets.UTF_8)));
+
+		assertThrows(SecurityException.class, () -> manager.save(fileUpload, "../escaped"));
+	}
+
+	@Test
+	void saveRejectsTraversalInClientFileName() throws IOException
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+		FileUpload fileUpload = mock(FileUpload.class);
+		when(fileUpload.getClientFileName()).thenReturn("../../etc/passwd");
+		when(fileUpload.getInputStream())
+			.thenReturn(new ByteArrayInputStream("content".getBytes(StandardCharsets.UTF_8)));
+
+		assertThrows(SecurityException.class, () -> manager.save(fileUpload, "uploadField"));
+	}
+
+	@Test
+	void saveSucceedsWithValidUploadFieldIdAndClientFileName() throws IOException
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+		FileUpload fileUpload = mock(FileUpload.class);
+		when(fileUpload.getClientFileName()).thenReturn("safe.txt");
+		when(fileUpload.getInputStream())
+			.thenReturn(new ByteArrayInputStream("content".getBytes(StandardCharsets.UTF_8)));
+
+		manager.save(fileUpload, "uploadField");
+
+		Path savedFile = tempDir.resolve("uploadField").resolve("safe.txt");
+		assertTrue(Files.exists(savedFile));
+		assertEquals("content", Files.readString(savedFile, StandardCharsets.UTF_8));
+	}
+}
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ClientSideImageMapTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ClientSideImageMapTest.java
index b69922f8a8..e53f30dc54 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ClientSideImageMapTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ClientSideImageMapTest.java
@@ -1,38 +1,38 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.wicket.markup.html.link;
-
-import java.util.Locale;
-
-import org.apache.wicket.util.tester.WicketTestCase;
-import org.junit.jupiter.api.Test;
-
-/**
- * @since 1.5
- */
-class ClientSideImageMapTest extends WicketTestCase
-{
-	/**
-	 * @throws Exception
-	 */
-	@Test
-    void testRenderClientSideImageMapPage_1() throws Exception
-	{
-		tester.getSession().setLocale(Locale.US);
-		executeTest(ClientSideImageMapPage_1.class, "ClientSideImageMapPageExpectedResult_1.html");
-	}
-}
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.markup.html.link;
+
+import java.util.Locale;
+
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.jupiter.api.Test;
+
+/**
+ * @since 1.5
+ */
+class ClientSideImageMapTest extends WicketTestCase
+{
+	/**
+	 * @throws Exception
+	 */
+	@Test
+    void testRenderClientSideImageMapPage_1() throws Exception
+	{
+		tester.getSession().setLocale(Locale.US);
+		executeTest(ClientSideImageMapPage_1.class, "ClientSideImageMapPageExpectedResult_1.html");
+	}
+}
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ExternalLinkTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ExternalLinkTest.java
index ffbcd67f50..e28f607f67 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ExternalLinkTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ExternalLinkTest.java
@@ -1,49 +1,110 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.wicket.markup.html.link;
-
-import org.apache.wicket.util.tester.WicketTestCase;
-import org.junit.jupiter.api.Test;
-
-/**
- * Test ExternalLink (href="...")
- * 
- * <a href="https://issues.apache.org/jira/browse/WICKET-1016"></<a>
- */
-class ExternalLinkTest extends WicketTestCase
-{
-	/**
-	 * @throws Exception
-	 */
-	@Test
-    void renderExternalLink_1() throws Exception
-	{
-		tester.getApplication().getMarkupSettings().setAutomaticLinking(true);
-		executeTest(ExternalLinkPage_1.class, "ExternalLinkPageExpectedResult_1.html");
-	}
-
-	/**
-	 * @throws Exception
-	 */
-	@Test
-    void renderExternalLink_2() throws Exception
-	{
-		tester.getApplication().getMarkupSettings().setAutomaticLinking(true);
-		executeTest(ExternalLinkPage_2.class, "ExternalLinkPageExpectedResult_2.html");
-	}
-
-}
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.markup.html.link;
+
+import org.apache.wicket.MockPageWithOneComponent;
+import org.apache.wicket.core.util.string.JavaScriptUtils;
+import org.apache.wicket.markup.ComponentTag;
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.jupiter.api.Test;
+
+import static org.apache.wicket.MockPageWithOneComponent.COMPONENT_ID;
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Test ExternalLink (href="...")
+ * 
+ * <a href="https://issues.apache.org/jira/browse/WICKET-1016"></<a>
+ */
+class ExternalLinkTest extends WicketTestCase
+{
+
+	@Test
+	void allowsJavascriptScheme() throws Exception
+	{
+		String uri = "javascript:alert(1)";
+		MockPageWithOneComponent page = new MockPageWithOneComponent();
+		page.add(new ExternalLink(COMPONENT_ID, uri){
+			@Override
+			protected void onComponentTag(ComponentTag tag)
+			{
+				super.onComponentTag(tag);
+				tag.setName("a");
+			}
+		});
+
+		tester.startPage(page);
+
+		assertThat(tester.getLastResponseAsString()).contains(uri);
+	}
+
+	@Test
+	void escapesJavascriptQuotes() throws Exception
+	{
+		String unescaped = "javascript:alert('foo')";
+		MockPageWithOneComponent page = new MockPageWithOneComponent();
+		page.add(new ExternalLink(COMPONENT_ID, unescaped));
+
+		tester.startPage(page);
+
+		assertThat(tester.getLastResponseAsString()).contains("javascript:alert(\\'foo\\')");
+	}
+
+	@Test
+	void allowsJavascriptSchemeInPopupTarget() throws Exception
+	{
+		String uri = "javascript:alert(1)";
+		MockPageWithOneComponent page = new MockPageWithOneComponent();
+		page.add(new ExternalLink(COMPONENT_ID, uri));
+
+		tester.startPage(page);
+
+		assertThat(tester.getLastResponseAsString()).contains(uri);
+	}
+	@Test
+	void escapeJavascriptQuotes() throws Exception
+	{
+		String uri = "javascript:alert('foo')";
+		MockPageWithOneComponent page = new MockPageWithOneComponent();
+		page.add(new ExternalLink(COMPONENT_ID, uri));
+
+		tester.startPage(page);
+
+		assertThat(tester.getLastResponseAsString()).contains(JavaScriptUtils.escapeQuotes(uri));
+	}
+
+	/**
+	 * @throws Exception
+	 */
+	@Test
+    void renderExternalLink_1() throws Exception
+	{
+		tester.getApplication().getMarkupSettings().setAutomaticLinking(true);
+		executeTest(ExternalLinkPage_1.class, "ExternalLinkPageExpectedResult_1.html");
+	}
+
+	/**
+	 * @throws Exception
+	 */
+	@Test
+    void renderExternalLink_2() throws Exception
+	{
+		tester.getApplication().getMarkupSettings().setAutomaticLinking(true);
+		executeTest(ExternalLinkPage_2.class, "ExternalLinkPageExpectedResult_2.html");
+	}
+
+}
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/LinkTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/LinkTest.java
new file mode 100644
index 0000000000..e5c432db63
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/LinkTest.java
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.markup.html.link;
+
+import org.apache.wicket.MockPageWithLink;
+import org.apache.wicket.MockPageWithOneComponent;
+import org.apache.wicket.WicketRuntimeException;
+import org.apache.wicket.markup.ComponentTag;
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.jupiter.api.Test;
+
+import static org.apache.wicket.MockPageWithOneComponent.COMPONENT_ID;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class LinkTest extends WicketTestCase
+{
+
+	@Test
+	void allowsJavascriptSchemeInPopupsTarget()
+	{
+		var uri = "javascript:alert(1);";
+		MockPageWithOneComponent page = new MockPageWithOneComponent();
+		page.add(new PopupLink(COMPONENT_ID, uri));
+
+		tester.startPage(page);
+
+		assertThat(tester.getLastResponseAsString()).contains(uri);
+	}
+
+	@Test
+	void escapesJavascriptQuotesInPopupsTarget()
+	{
+		var uri = "javascript:alert('foo');";
+		MockPageWithOneComponent page = new MockPageWithOneComponent();
+		page.add(new PopupLink(COMPONENT_ID, uri));
+
+		tester.startPage(page);
+
+		assertThat(tester.getLastResponseAsString()).contains("javascript:alert(\\'foo\\');");
+	}
+
+	@Test
+	void testWrongComponentId()
+	{
+		MockPageWithLink mockPageWithLink = new MockPageWithLink();
+		Link<Void> link = new Link<Void>("linkx")
+		{
+			private static final long serialVersionUID = 1L;
+
+			@Override
+			public void onClick()
+			{
+			}
+
+		};
+
+		mockPageWithLink.add(link);
+		assertThrows(WicketRuntimeException.class, () -> tester.startPage(mockPageWithLink));
+	}
+
+	static class PopupLink extends Link<Void>
+	{
+		private final String uri;
+
+		public PopupLink(String id, String uri)
+		{
+			super(id);
+			this.uri = uri;
+			setPopupSettings(new PopupSettings());
+		}
+
+		@Override
+		public void onClick()
+		{
+		}
+
+		@Override
+		protected void onComponentTag(ComponentTag tag)
+		{
+			super.onComponentTag(tag);
+			tag.setName("a");
+		}
+
+		@Override
+		protected CharSequence getURL()
+		{
+			return uri;
+		}
+	}
+
+}
\ No newline at end of file
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/snake_case/TestPageInsideSnakeCasePackage.html b/wicket-core/src/test/java/org/apache/wicket/markup/html/snake_case/TestPageInsideSnakeCasePackage.html
new file mode 100644
index 0000000000..ea75f28073
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/snake_case/TestPageInsideSnakeCasePackage.html
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html>
+<html xmlns:wicket="http://wicket.apache.org">
+<head>
+    <meta http-equiv="Content-Type" content="text/html;charset=UTF-8"/>
+    <title>Test Page</title>
+</head>
+<body>
+none
+</body>
+</html>
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ClientSideImageMapTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/snake_case/TestPageInsideSnakeCasePackage.java
similarity index 61%
copy from wicket-core/src/test/java/org/apache/wicket/markup/html/link/ClientSideImageMapTest.java
copy to wicket-core/src/test/java/org/apache/wicket/markup/html/snake_case/TestPageInsideSnakeCasePackage.java
index b69922f8a8..ee3288eaa8 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ClientSideImageMapTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/snake_case/TestPageInsideSnakeCasePackage.java
@@ -1,38 +1,23 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.wicket.markup.html.link;
-
-import java.util.Locale;
-
-import org.apache.wicket.util.tester.WicketTestCase;
-import org.junit.jupiter.api.Test;
-
-/**
- * @since 1.5
- */
-class ClientSideImageMapTest extends WicketTestCase
-{
-	/**
-	 * @throws Exception
-	 */
-	@Test
-    void testRenderClientSideImageMapPage_1() throws Exception
-	{
-		tester.getSession().setLocale(Locale.US);
-		executeTest(ClientSideImageMapPage_1.class, "ClientSideImageMapPageExpectedResult_1.html");
-	}
-}
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.markup.html.snake_case;
+
+import org.apache.wicket.markup.html.WebPage;
+
+public class TestPageInsideSnakeCasePackage extends WebPage
+{
+}
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/snake_case/style.css b/wicket-core/src/test/java/org/apache/wicket/markup/html/snake_case/style.css
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRenderer.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRenderer.java
index 51186a5460..b6755ab1e9 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRenderer.java
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRenderer.java
@@ -16,6 +16,7 @@
  */
 package org.apache.wicket.extensions.ajax.markup.html.autocomplete;
 
+import org.apache.wicket.core.util.string.JavaScriptUtils;
 import org.apache.wicket.request.Response;
 import org.apache.wicket.util.string.Strings;
 
@@ -50,7 +51,7 @@ public abstract class AbstractAutoCompleteRenderer<T> implements IAutoCompleteRe
 		final CharSequence handler = getOnSelectJavaScriptExpression(object);
 		if (handler != null)
 		{
-			response.write(" onselect=\"" + handler + '"');
+			response.write(" onselect=\"" + Strings.escapeMarkup(handler) + '"');
 		}
 		response.write(">");
 		renderChoice(object, response, criteria);
diff --git a/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRendererTest.java b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRendererTest.java
new file mode 100644
index 0000000000..59e80ab651
--- /dev/null
+++ b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRendererTest.java
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.extensions.ajax.markup.html.autocomplete;
+
+import org.apache.wicket.mock.MockWebResponse;
+import org.apache.wicket.request.Response;
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class AbstractAutoCompleteRendererTest extends WicketTestCase
+{
+	@Test
+	void escapeOnselectJSExpression()
+	{
+		var renderer = new Renderer("alert('hello');");
+		MockWebResponse response = new MockWebResponse();
+		renderer.render("foo", response, null);
+		Assertions.assertTrue(response.getTextResponse().toString()
+			.contains("<li textvalue=\"foo\" onselect=\"alert(&#039;hello&#039;);\">foo</li>"));
+	}
+
+	static class Renderer extends AbstractAutoCompleteRenderer<String>
+	{
+
+		final String expression;
+
+		Renderer(String expression)
+		{
+			this.expression = expression;
+		}
+
+		@Override
+		protected CharSequence getOnSelectJavaScriptExpression(String item)
+		{
+			return expression;
+		}
+
+		@Override
+		protected void renderChoice(String object, Response response, String criteria)
+		{
+			response.write(object);
+		}
+
+		@Override
+		protected String getTextValue(String object)
+		{
+			return object;
+		}
+	}
+}