Skip to content

Commit

Permalink
Follow P2 contract of cached file's extension (eclipse-tycho#2938)
Browse files Browse the repository at this point in the history
P2 relies on correct file extensions to parse cached files.

Fixes eclipse-tycho#2938

See P2 bug:
eclipse-equinox/p2#355

Integration test infrastructure touch-ups:

Fix HttpServer concurrency bug.
URLs returned from HttpServer.getAccessedUrls() are now stripped of
context prefix. No callers have used these values until now.
Fix concurrency bug in test utility class HttpServer request logging.
  • Loading branch information
basilevs committed Oct 21, 2023
1 parent c1842b2 commit 31f701b
Show file tree
Hide file tree
Showing 9 changed files with 274 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions tycho-its/projects/target.content_jar/category.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="UTF-8"?>
<site>
<feature id="issue_2938_reproducer"/>
</site>
49 changes: 49 additions & 0 deletions tycho-its/projects/target.content_jar/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>tycho-its-project.p2Repository.slicerDependencies</groupId>
<artifactId>aggregator</artifactId>
<version>1.0.0</version>
<packaging>eclipse-repository</packaging>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-maven-plugin</artifactId>
<version>${tycho-version}</version>
<extensions>true</extensions>
</plugin>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>target-platform-configuration</artifactId>
<version>${tycho-version}</version>
<configuration>
<resolver>p2</resolver>
<executionEnvironment>JavaSE-11</executionEnvironment>
<target>
<file>targetplatform.target</file>
</target>
<environments>
<environment>
<os>win32</os>
<ws>win32</ws>
<arch>x86_64</arch>
</environment>
<environment>
<os>linux</os>
<ws>gtk</ws>
<arch>x86_64</arch>
</environment>
</environments>
</configuration>
</plugin>
</plugins>
</build>

</project>
11 changes: 11 additions & 0 deletions tycho-its/projects/target.content_jar/targetplatform.target
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<?pde?>
<target name="p2Repository.slicerDependencies" sequenceNumber="1">
<locations>
<location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="slicer" includeSource="true" type="InstallableUnit">
<repository id="repoA" location="url"/>

<unit id="issue_2938_reproducer.feature.group" version="0.0.0"/>
</location>
</locations>
</target>
Binary file added tycho-its/repositories/content_jar/artifacts.jar
Binary file not shown.
Binary file added tycho-its/repositories/content_jar/content.jar
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -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<String> 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<Path>() {
@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);
}

}
103 changes: 70 additions & 33 deletions tycho-its/src/test/java/org/eclipse/tycho/test/util/HttpServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<String> 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<String> 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<String, String> relativeUrlToNewUrl;

public RedirectServlet(Function<String, String> 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;
Expand All @@ -80,7 +96,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)

private final int port;

private final Map<String, MonitoringServlet> contextName2servletsMap = new HashMap<>();
private final MultiMap<String> contextName2accessedUrls = new MultiMap<>();

private ContextHandlerCollection contexts;

Expand Down Expand Up @@ -153,26 +169,47 @@ 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<String, String> 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) {
return "http://localhost:" + port + "/" + contextName;
}

public List<String> 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);
}
}
}

0 comments on commit 31f701b

Please sign in to comment.