Bump Apache Velocity to 2.3 Apache Velocity 2.3 is the latest version and relies on commons lang v3 as Gerrit 3.6 requires. Also use the new artifact name velocity-engine-core and use the Gerrit's static assets servlet for serving binary files as the new Velocity resource manager in v2 isn't able to read binary content. Inject the Plugin instance for allowing the access and load of the plugin's own resources. Bug: Issue 15897 Change-Id: I5802c16167d9a0b8591cd42e0a7e9ef429e4d5a3
diff --git a/github-plugin/pom.xml b/github-plugin/pom.xml index e12fd99..4dce2e4 100644 --- a/github-plugin/pom.xml +++ b/github-plugin/pom.xml
@@ -160,8 +160,8 @@ </dependency> <dependency> <groupId>org.apache.velocity</groupId> - <artifactId>velocity</artifactId> - <version>1.7</version> + <artifactId>velocity-engine-core</artifactId> + <version>2.3</version> </dependency> <dependency> <groupId>org.jukito</groupId>
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/notification/WebhookServlet.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/notification/WebhookServlet.java index 01fe742..1bdce3c 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/notification/WebhookServlet.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/notification/WebhookServlet.java
@@ -53,7 +53,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.commons.codec.DecoderException; import org.apache.commons.codec.binary.Hex; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory;
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityModel.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityModel.java index 4970e1d..87fe39d 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityModel.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityModel.java
@@ -43,7 +43,7 @@ return context.put(key, value); } - public Object remove(Object key) { + public Object remove(String key) { return context.remove(key); } }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityRuntimeProvider.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityRuntimeProvider.java index d0080c5..e1b4d9b 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityRuntimeProvider.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityRuntimeProvider.java
@@ -24,7 +24,6 @@ import java.util.Properties; import org.apache.velocity.runtime.RuntimeConstants; import org.apache.velocity.runtime.RuntimeInstance; -import org.apache.velocity.runtime.log.Log4JLogChute; import org.apache.velocity.runtime.resource.loader.ClasspathResourceLoader; import org.apache.velocity.runtime.resource.loader.JarResourceLoader; @@ -51,7 +50,6 @@ Properties p = new Properties(); p.setProperty(RuntimeConstants.VM_PERM_INLINE_LOCAL, "true"); - p.setProperty(RuntimeConstants.RUNTIME_LOG_LOGSYSTEM_CLASS, Log4JLogChute.class.getName()); p.setProperty(RuntimeConstants.RUNTIME_REFERENCES_STRICT, "true"); p.setProperty("runtime.log.logsystem.log4j.category", "velocity");
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityStaticServlet.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityStaticServlet.java index 06a5556..7ea0283 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityStaticServlet.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityStaticServlet.java
@@ -15,21 +15,26 @@ package com.googlesource.gerrit.plugins.github.velocity; import com.google.common.collect.Maps; +import com.google.gerrit.httpd.raw.SiteStaticDirectoryServlet; +import com.google.gerrit.server.plugins.Plugin; +import com.google.gerrit.server.plugins.PluginEntry; import com.google.gerrit.util.http.CacheHeaders; import com.google.gerrit.util.http.RequestUtil; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; import com.google.inject.name.Named; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; +import java.io.*; +import java.nio.charset.StandardCharsets; import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.zip.GZIPOutputStream; +import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; import org.apache.commons.io.IOUtils; import org.apache.velocity.runtime.RuntimeInstance; @@ -43,25 +48,17 @@ public class VelocityStaticServlet extends HttpServlet { private static final Logger log = LoggerFactory.getLogger(VelocityStaticServlet.class); private static final Map<String, String> MIME_TYPES = Maps.newHashMap(); + private static final String STATIC_PATH_PREFIX = "static/"; static { MIME_TYPES.put("html", "text/html"); MIME_TYPES.put("htm", "text/html"); MIME_TYPES.put("js", "application/x-javascript"); MIME_TYPES.put("css", "text/css"); - MIME_TYPES.put("rtf", "text/rtf"); - MIME_TYPES.put("txt", "text/plain"); - MIME_TYPES.put("text", "text/plain"); - MIME_TYPES.put("pdf", "application/pdf"); - MIME_TYPES.put("jpeg", "image/jpeg"); - MIME_TYPES.put("jpg", "image/jpeg"); - MIME_TYPES.put("gif", "image/gif"); - MIME_TYPES.put("png", "image/png"); - MIME_TYPES.put("tiff", "image/tiff"); - MIME_TYPES.put("tif", "image/tiff"); - MIME_TYPES.put("svg", "image/svg+xml"); } + private static final Set<String> VELOCITY_STATIC_TYPES = Set.of("html", "htm", "js", "css"); + private static String contentType(final String name) { final int dot = name.lastIndexOf('.'); final String ext = 0 < dot ? name.substring(dot + 1) : ""; @@ -69,14 +66,35 @@ return type != null ? type : "application/octet-stream"; } + private byte[] read(PluginEntry pluginEntry) throws IOException { + try (InputStream raw = plugin.getContentScanner().getInputStream(pluginEntry)) { + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + IOUtils.copy(raw, out); + out.flush(); + return out.toByteArray(); + } + } + private static byte[] readResource(final Resource p) throws IOException { - try (InputStream in = p.getResourceLoader().getResourceStream(p.getName()); + try (Reader in = + p.getResourceLoader().getResourceReader(p.getName(), StandardCharsets.UTF_8.name()); ByteArrayOutputStream byteOut = new ByteArrayOutputStream()) { IOUtils.copy(in, byteOut); return byteOut.toByteArray(); } } + private byte[] compress(PluginEntry pluginEntry) throws IOException { + try (InputStream raw = plugin.getContentScanner().getInputStream(pluginEntry)) { + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final GZIPOutputStream gz = new GZIPOutputStream(out); + IOUtils.copy(raw, gz); + gz.finish(); + gz.flush(); + return out.toByteArray(); + } + } + private static byte[] compress(final byte[] raw) throws IOException { final ByteArrayOutputStream out = new ByteArrayOutputStream(); final GZIPOutputStream gz = new GZIPOutputStream(out); @@ -87,16 +105,22 @@ } private final RuntimeInstance velocity; + private final SiteStaticDirectoryServlet siteStaticServlet; + private final Plugin plugin; @Inject VelocityStaticServlet( - @Named("PluginRuntimeInstance") final Provider<RuntimeInstance> velocityRuntimeProvider) { + @Named("PluginRuntimeInstance") final Provider<RuntimeInstance> velocityRuntimeProvider, + SiteStaticDirectoryServlet siteStaticServlet, + Plugin plugin) { this.velocity = velocityRuntimeProvider.get(); + this.siteStaticServlet = siteStaticServlet; + this.plugin = plugin; } private Resource local(final HttpServletRequest req) { final String name = req.getPathInfo(); - if (name.length() < 2 || !name.startsWith("/") || isUnreasonableName(name)) { + if (name.length() < 2 || !name.startsWith("/")) { // Too short to be a valid file name, or doesn't start with // the path info separator like we expected. // @@ -112,6 +136,24 @@ } } + private boolean isVelocityStaticResource(String resourceName) { + final int dot = resourceName.lastIndexOf('.'); + final String ext = 0 < dot ? resourceName.substring(dot + 1) : ""; + return VELOCITY_STATIC_TYPES.contains(ext.toLowerCase()); + } + + private String resourceName(HttpServletRequest req) { + final String name = req.getPathInfo(); + if (name.length() < 2 || !name.startsWith("/") || isUnreasonableName(name)) { + // Too short to be a valid file name, or doesn't start with + // the path info separator like we expected. + // + return null; + } + + return name.substring(1); + } + private static boolean isUnreasonableName(String name) { if (name.charAt(name.length() - 1) == '/') return true; // no suffix if (name.indexOf('\\') >= 0) return true; // no windows/dos stlye paths @@ -131,29 +173,68 @@ @Override protected void doGet(final HttpServletRequest req, final HttpServletResponse rsp) - throws IOException { - final Resource p = local(req); - if (p == null) { + throws IOException, ServletException { + String resourceName = resourceName(req); + if (isUnreasonableName(resourceName) || !resourceName.startsWith(STATIC_PATH_PREFIX)) { CacheHeaders.setNotCacheable(rsp); rsp.setStatus(HttpServletResponse.SC_NOT_FOUND); return; } - final String type = contentType(p.getName()); - final byte[] tosend; - if (!type.equals("application/x-javascript") && RequestUtil.acceptsGzipEncoding(req)) { - rsp.setHeader("Content-Encoding", "gzip"); - tosend = compress(readResource(p)); - } else { - tosend = readResource(p); + if (isVelocityStaticResource(resourceName)) { + final Resource p = local(req); + if (p == null) { + CacheHeaders.setNotCacheable(rsp); + rsp.setStatus(HttpServletResponse.SC_NOT_FOUND); + return; + } + + final String type = contentType(p.getName()); + final byte[] tosend; + if (!type.equals("application/x-javascript") && RequestUtil.acceptsGzipEncoding(req)) { + rsp.setHeader("Content-Encoding", "gzip"); + tosend = compress(readResource(p)); + } else { + tosend = readResource(p); + } + + CacheHeaders.setCacheable(req, rsp, 12, TimeUnit.HOURS); + rsp.setDateHeader("Last-Modified", p.getLastModified()); + rsp.setContentType(type); + rsp.setContentLength(tosend.length); + try (OutputStream out = rsp.getOutputStream()) { + out.write(tosend); + } } - CacheHeaders.setCacheable(req, rsp, 12, TimeUnit.HOURS); - rsp.setDateHeader("Last-Modified", p.getLastModified()); - rsp.setContentType(type); - rsp.setContentLength(tosend.length); - try (OutputStream out = rsp.getOutputStream()) { - out.write(tosend); + Optional<PluginEntry> jarResource = plugin.getContentScanner().getEntry(resourceName); + if (jarResource.isPresent()) { + final String type = contentType(resourceName); + final byte[] tosend; + if (!type.equals("application/x-javascript") && RequestUtil.acceptsGzipEncoding(req)) { + rsp.setHeader("Content-Encoding", "gzip"); + tosend = compress(jarResource.get()); + } else { + tosend = read(jarResource.get()); + } + + CacheHeaders.setCacheable(req, rsp, 12, TimeUnit.HOURS); + rsp.setContentType(type); + rsp.setContentLength(tosend.length); + try (OutputStream out = rsp.getOutputStream()) { + out.write(tosend); + } + } else { + + HttpServletRequestWrapper mappedReq = + new HttpServletRequestWrapper(req) { + + @Override + public String getPathInfo() { + return super.getPathInfo().substring(STATIC_PATH_PREFIX.length()); + } + }; + siteStaticServlet.service(mappedReq, rsp); } } }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java index 23edcbc..fc578af 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java
@@ -51,7 +51,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.eclipse.jgit.errors.ConfigInvalidException; import org.kohsuke.github.GHKey; import org.kohsuke.github.GHMyself;
diff --git a/github-plugin/src/main/resources/static/scope.html b/github-plugin/src/main/resources/static/scope.html index 5b46f4c..113b5b5 100644 --- a/github-plugin/src/main/resources/static/scope.html +++ b/github-plugin/src/main/resources/static/scope.html
@@ -55,7 +55,7 @@ #set ( $scopeItems = $config.scopes.get($scope) ) #foreach ( $scopeItem in $scopeItems ) $scopeItem.description - #if ( $velocityCount < $scopeItems.size()) + #if ( $foreach.count < $scopeItems.size()) , #end #end