diff --git a/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/SharedHttpCacheStorage.java b/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/SharedHttpCacheStorage.java index ca3e8cc54d..32237fc0e8 100644 --- a/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/SharedHttpCacheStorage.java +++ b/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/SharedHttpCacheStorage.java @@ -262,8 +262,16 @@ public synchronized File fetchFile(URI uri, HttpTransportFactory transportFactor } updateHeader(response, code); if (isRedirected(code)) { - return SharedHttpCacheStorage.this.getCacheEntry(getRedirect(uri), logger) + File cachedFile = SharedHttpCacheStorage.this.getCacheEntry(getRedirect(uri), logger) .getCacheFile(transportFactory); + // https://github.com/eclipse-tycho/tycho/issues/2938 + // Redirect may change extension. P2's SimpleMetadataRepositoryFactory relies on + // accurate file extension to be cached. + // Copying file to accommodate original request and its file extension. + // Once https://github.com/eclipse-equinox/p2/issues/355 is fixed, cachedFile + // may be returned directly without copying. + FileUtils.copyFile(cachedFile, file); + return file; } if (exits) { FileUtils.forceDelete(file); diff --git a/tycho-its/projects/target.content_jar/category.xml b/tycho-its/projects/target.content_jar/category.xml new file mode 100644 index 0000000000..7eaddf9158 --- /dev/null +++ b/tycho-its/projects/target.content_jar/category.xml @@ -0,0 +1,4 @@ + + + + diff --git a/tycho-its/projects/target.content_jar/pom.xml b/tycho-its/projects/target.content_jar/pom.xml new file mode 100644 index 0000000000..a90b318d2e --- /dev/null +++ b/tycho-its/projects/target.content_jar/pom.xml @@ -0,0 +1,49 @@ + + 4.0.0 + tycho-its-project.p2Repository.slicerDependencies + aggregator + 1.0.0 + eclipse-repository + + + UTF-8 + + + + + + org.eclipse.tycho + tycho-maven-plugin + ${tycho-version} + true + + + org.eclipse.tycho + target-platform-configuration + ${tycho-version} + + p2 + JavaSE-11 + + targetplatform.target + + + + win32 + win32 + x86_64 + + + linux + gtk + x86_64 + + + + + + + + diff --git a/tycho-its/projects/target.content_jar/targetplatform.target b/tycho-its/projects/target.content_jar/targetplatform.target new file mode 100644 index 0000000000..1f4ffb4d15 --- /dev/null +++ b/tycho-its/projects/target.content_jar/targetplatform.target @@ -0,0 +1,11 @@ + + + + + + + + + + + \ No newline at end of file diff --git a/tycho-its/repositories/content_jar/artifacts.jar b/tycho-its/repositories/content_jar/artifacts.jar new file mode 100644 index 0000000000..5ea5212851 Binary files /dev/null and b/tycho-its/repositories/content_jar/artifacts.jar differ diff --git a/tycho-its/repositories/content_jar/content.jar b/tycho-its/repositories/content_jar/content.jar new file mode 100644 index 0000000000..a69c2002cd Binary files /dev/null and b/tycho-its/repositories/content_jar/content.jar differ diff --git a/tycho-its/repositories/content_jar/features/issue_2938_reproducer_1.0.0.202310211419.jar b/tycho-its/repositories/content_jar/features/issue_2938_reproducer_1.0.0.202310211419.jar new file mode 100644 index 0000000000..9c1b4111ba Binary files /dev/null and b/tycho-its/repositories/content_jar/features/issue_2938_reproducer_1.0.0.202310211419.jar differ diff --git a/tycho-its/src/test/java/org/eclipse/tycho/test/tycho2938/ContentJarTest.java b/tycho-its/src/test/java/org/eclipse/tycho/test/tycho2938/ContentJarTest.java new file mode 100644 index 0000000000..2d3f93b2b1 --- /dev/null +++ b/tycho-its/src/test/java/org/eclipse/tycho/test/tycho2938/ContentJarTest.java @@ -0,0 +1,131 @@ +/******************************************************************************* + * Copyright (c) 2023, 2023 Sonatype Inc. and others. + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Vasili Gulevich - initial implementation + *******************************************************************************/ +package org.eclipse.tycho.test.tycho2938; + +import java.io.File; +import java.io.IOException; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.List; + +import javax.xml.parsers.ParserConfigurationException; + +import org.apache.commons.io.FileUtils; +import org.apache.maven.it.VerificationException; +import org.apache.maven.it.Verifier; +import org.eclipse.tycho.test.AbstractTychoIntegrationTest; +import org.eclipse.tycho.test.util.HttpServer; +import org.eclipse.tycho.test.util.ResourceUtil; +import org.eclipse.tycho.test.util.TargetDefinitionUtil; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.xml.sax.SAXException; + +public class ContentJarTest extends AbstractTychoIntegrationTest { + private HttpServer server; + private static final String TARGET_FEATURE_PATH = "/features/issue_2938_reproducer_1.0.0.202310211419.jar"; + @Rule + public final TemporaryFolder temporaryFolder = new TemporaryFolder(); + private Verifier verifier; + private String mainRepoUrl; + + @Before + public void startServer() throws Exception { + server = HttpServer.startServer(); + File originalResource = ResourceUtil.resolveTestResource("repositories/content_jar"); + FileUtils.copyDirectory(originalResource, temporaryFolder.getRoot()); + verifier = getVerifier("target.content_jar", false); + verifier.deleteArtifacts("p2.org.eclipse.update.feature", "issue_2938_reproducer", "1.0.0.202310211419"); + File repositoryRoot = temporaryFolder.getRoot(); + this.mainRepoUrl = server.addServer("repoA", repositoryRoot); + } + + @After + public void stopServer() throws Exception { + if (server != null) { + server.stop(); + } + } + + @Test + public void noRedirect() throws Exception { + configureRepositoryInTargetDefinition(mainRepoUrl); + verifier.executeGoal("package"); + assertVisited(TARGET_FEATURE_PATH); + verifier.verifyErrorFreeLog(); + } + + @Test + public void redirectKeepFilename() throws Exception { + String redirectedUrl = server.addRedirect("repoB", originalPath -> mainRepoUrl + originalPath); + configureRepositoryInTargetDefinition(redirectedUrl); + verifier.executeGoal("package"); + assertVisited(TARGET_FEATURE_PATH); + verifier.verifyErrorFreeLog(); + } + + @Test + public void redirectToBadLocation() throws Exception { + String redirectedUrl = server.addRedirect("repoB", originalPath -> mainRepoUrl + originalPath + "_invalid"); + configureRepositoryInTargetDefinition(redirectedUrl); + Assert.assertThrows(VerificationException.class, () -> verifier.executeGoal("package")); + assertVisited("/content.jar_invalid"); + verifier.verifyTextInLog("No repository found at " + redirectedUrl); + } + + @Test + public void redirectToMangledLocations() throws Exception { + File repositoryRoot = temporaryFolder.getRoot(); + mangleFileNames(repositoryRoot.toPath()); + + // https://github.com/eclipse-tycho/tycho/issues/2938 + // Redirect may change extension. + String redirectedUrl = server.addRedirect("repoB", originalPath -> mainRepoUrl + originalPath + "_invalid"); + + configureRepositoryInTargetDefinition(redirectedUrl); + verifier.executeGoal("package"); + assertVisited("/content.jar_invalid"); + assertVisited(TARGET_FEATURE_PATH + "_invalid"); + verifier.verifyErrorFreeLog(); + } + + private void assertVisited(String path) { + List accessedUrls = server.getAccessedUrls("repoA"); + Assert.assertTrue(String.format("Path %s should be visited, %s were visited instead", path, accessedUrls), + accessedUrls.contains(path)); + } + + private void mangleFileNames(Path repositoryRoot) throws IOException { + Files.walkFileTree(repositoryRoot, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + Files.move(file, file.getParent().resolve(file.getFileName() + "_invalid")); + return super.visitFile(file, attrs); + } + }); + } + + private void configureRepositoryInTargetDefinition(String url) + throws IOException, ParserConfigurationException, SAXException { + File platformFile = new File(verifier.getBasedir(), "targetplatform.target"); + TargetDefinitionUtil.setRepositoryURLs(platformFile, "repoA", url); + } + +} diff --git a/tycho-its/src/test/java/org/eclipse/tycho/test/util/HttpServer.java b/tycho-its/src/test/java/org/eclipse/tycho/test/util/HttpServer.java index 682e6fb942..a67fedffa3 100644 --- a/tycho-its/src/test/java/org/eclipse/tycho/test/util/HttpServer.java +++ b/tycho-its/src/test/java/org/eclipse/tycho/test/util/HttpServer.java @@ -14,13 +14,19 @@ import java.io.File; import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; +import java.util.EnumSet; import java.util.List; -import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import javax.servlet.DispatcherType; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -35,43 +41,53 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.servlet.DefaultServlet; +import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.security.Password; public class HttpServer { - private static class MonitoringServlet extends DefaultServlet { - private List accessedURIs = new ArrayList<>(); + private class Monitoring implements Filter { @Override - public String getInitParameter(String name) { - // no directory listing allowed - if ("dirAllowed".equals(name)) { - return "false"; - } else { - return super.getInitParameter(name); - } + public void init(FilterConfig filterConfig) throws ServletException { } - public List getAccessedURIs() { - return accessedURIs; + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + Request httprequest = Request.getBaseRequest(request); + String context = httprequest.getContextPath(); + assert context.startsWith("/"); + context = context.substring(1); + synchronized (contextName2accessedUrls) { + contextName2accessedUrls.add(context, httprequest.getServletPath()); + } + chain.doFilter(request, response); } @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { - accessedURIs.add(((Request) request).getHttpURI().toString()); - super.doGet(request, response); + public void destroy() { + + } + } + + private static class RedirectServlet extends HttpServlet { + private final Function relativeUrlToNewUrl; + + public RedirectServlet(Function relativeUrlToNewUrl2) { + super(); + this.relativeUrlToNewUrl = relativeUrlToNewUrl2; } @Override - protected void doPost(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { - accessedURIs.add(((Request) request).getHttpURI().toString()); - super.doPost(request, response); + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + resp.sendRedirect(relativeUrlToNewUrl.apply(req.getServletPath())); } + } private static final int BIND_ATTEMPTS = 20; @@ -80,7 +96,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) private final int port; - private final Map contextName2servletsMap = new HashMap<>(); + private final MultiMap contextName2accessedUrls = new MultiMap<>(); private ContextHandlerCollection contexts; @@ -153,18 +169,28 @@ public void stop() throws Exception { public String addServer(String contextName, final File content) { ServletContextHandler context = new ServletContextHandler(contexts, URIUtil.SLASH + contextName); + context.setInitParameter("org.eclipse.jetty.servlet.Default.dirAllowed", "false"); context.setResourceBase(content.getAbsolutePath()); + context.addServlet(DefaultServlet.class, URIUtil.SLASH); + registerContext(context); + return getUrl(contextName); + } - MonitoringServlet monitoringServlet = new MonitoringServlet(); - contextName2servletsMap.put(contextName, monitoringServlet); - context.addServlet(new ServletHolder(monitoringServlet), URIUtil.SLASH); - contexts.addHandler(context); - try { - context.start(); - } catch (Exception e) { - throw new RuntimeException(e); - } + /** + * + * @param contextName - a path prefix to handle + * @param relativeUrlToNewUrl - redirecting function. Takes a path within + * context starting with slash and return an new + * absolute URL to redirect to (Location header + * value). + * @return an URL prefix to redirect from + */ + public String addRedirect(String contextName, Function relativeUrlToNewUrl) { + ServletContextHandler context = new ServletContextHandler(contexts, URIUtil.SLASH + contextName); + context.addServlet(new ServletHolder(new RedirectServlet(relativeUrlToNewUrl)), URIUtil.SLASH); + registerContext(context); return getUrl(contextName); + } public String getUrl(String contextName) { @@ -172,7 +198,18 @@ public String getUrl(String contextName) { } public List getAccessedUrls(String contextName) { - return contextName2servletsMap.get(contextName).getAccessedURIs(); + synchronized (contextName2accessedUrls) { + return List.copyOf(contextName2accessedUrls.get(contextName)); + } } + private void registerContext(ServletContextHandler context) { + context.addFilter(new FilterHolder(new Monitoring()), "*", EnumSet.of(DispatcherType.REQUEST)); + contexts.addHandler(context); + try { + context.start(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } }