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
