Skip to content

Commit fe69296

Browse files
committed
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
1 parent 89f925b commit fe69296

File tree

7 files changed

+121
-42
lines changed

7 files changed

+121
-42
lines changed

github-plugin/pom.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ limitations under the License.
160160
</dependency>
161161
<dependency>
162162
<groupId>org.apache.velocity</groupId>
163-
<artifactId>velocity</artifactId>
164-
<version>1.7</version>
163+
<artifactId>velocity-engine-core</artifactId>
164+
<version>2.3</version>
165165
</dependency>
166166
<dependency>
167167
<groupId>org.jukito</groupId>

github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/notification/WebhookServlet.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
import javax.servlet.http.HttpServletResponse;
5454
import org.apache.commons.codec.DecoderException;
5555
import org.apache.commons.codec.binary.Hex;
56-
import org.apache.commons.lang.StringUtils;
56+
import org.apache.commons.lang3.StringUtils;
5757
import org.slf4j.Logger;
5858
import org.slf4j.LoggerFactory;
5959

github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityModel.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public Object put(String key, Object value) {
4343
return context.put(key, value);
4444
}
4545

46-
public Object remove(Object key) {
46+
public Object remove(String key) {
4747
return context.remove(key);
4848
}
4949
}

github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityRuntimeProvider.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import java.util.Properties;
2525
import org.apache.velocity.runtime.RuntimeConstants;
2626
import org.apache.velocity.runtime.RuntimeInstance;
27-
import org.apache.velocity.runtime.log.Log4JLogChute;
2827
import org.apache.velocity.runtime.resource.loader.ClasspathResourceLoader;
2928
import org.apache.velocity.runtime.resource.loader.JarResourceLoader;
3029

@@ -51,7 +50,6 @@ public RuntimeInstance get() {
5150

5251
Properties p = new Properties();
5352
p.setProperty(RuntimeConstants.VM_PERM_INLINE_LOCAL, "true");
54-
p.setProperty(RuntimeConstants.RUNTIME_LOG_LOGSYSTEM_CLASS, Log4JLogChute.class.getName());
5553
p.setProperty(RuntimeConstants.RUNTIME_REFERENCES_STRICT, "true");
5654
p.setProperty("runtime.log.logsystem.log4j.category", "velocity");
5755

github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityStaticServlet.java

Lines changed: 115 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,26 @@
1515
package com.googlesource.gerrit.plugins.github.velocity;
1616

1717
import com.google.common.collect.Maps;
18+
import com.google.gerrit.httpd.raw.SiteStaticDirectoryServlet;
19+
import com.google.gerrit.server.plugins.Plugin;
20+
import com.google.gerrit.server.plugins.PluginEntry;
1821
import com.google.gerrit.util.http.CacheHeaders;
1922
import com.google.gerrit.util.http.RequestUtil;
2023
import com.google.inject.Inject;
2124
import com.google.inject.Provider;
2225
import com.google.inject.Singleton;
2326
import com.google.inject.name.Named;
24-
import java.io.ByteArrayOutputStream;
25-
import java.io.IOException;
26-
import java.io.InputStream;
27-
import java.io.OutputStream;
27+
import java.io.*;
28+
import java.nio.charset.StandardCharsets;
2829
import java.util.Map;
30+
import java.util.Optional;
31+
import java.util.Set;
2932
import java.util.concurrent.TimeUnit;
3033
import java.util.zip.GZIPOutputStream;
34+
import javax.servlet.ServletException;
3135
import javax.servlet.http.HttpServlet;
3236
import javax.servlet.http.HttpServletRequest;
37+
import javax.servlet.http.HttpServletRequestWrapper;
3338
import javax.servlet.http.HttpServletResponse;
3439
import org.apache.commons.io.IOUtils;
3540
import org.apache.velocity.runtime.RuntimeInstance;
@@ -43,40 +48,53 @@
4348
public class VelocityStaticServlet extends HttpServlet {
4449
private static final Logger log = LoggerFactory.getLogger(VelocityStaticServlet.class);
4550
private static final Map<String, String> MIME_TYPES = Maps.newHashMap();
51+
private static final String STATIC_PATH_PREFIX = "static/";
4652

4753
static {
4854
MIME_TYPES.put("html", "text/html");
4955
MIME_TYPES.put("htm", "text/html");
5056
MIME_TYPES.put("js", "application/x-javascript");
5157
MIME_TYPES.put("css", "text/css");
52-
MIME_TYPES.put("rtf", "text/rtf");
53-
MIME_TYPES.put("txt", "text/plain");
54-
MIME_TYPES.put("text", "text/plain");
55-
MIME_TYPES.put("pdf", "application/pdf");
56-
MIME_TYPES.put("jpeg", "image/jpeg");
57-
MIME_TYPES.put("jpg", "image/jpeg");
58-
MIME_TYPES.put("gif", "image/gif");
59-
MIME_TYPES.put("png", "image/png");
60-
MIME_TYPES.put("tiff", "image/tiff");
61-
MIME_TYPES.put("tif", "image/tiff");
62-
MIME_TYPES.put("svg", "image/svg+xml");
6358
}
6459

60+
private static final Set<String> VELOCITY_STATIC_TYPES = Set.of("html", "htm", "js", "css");
61+
6562
private static String contentType(final String name) {
6663
final int dot = name.lastIndexOf('.');
6764
final String ext = 0 < dot ? name.substring(dot + 1) : "";
6865
final String type = MIME_TYPES.get(ext);
6966
return type != null ? type : "application/octet-stream";
7067
}
7168

69+
private byte[] read(PluginEntry pluginEntry) throws IOException {
70+
try (InputStream raw = plugin.getContentScanner().getInputStream(pluginEntry)) {
71+
final ByteArrayOutputStream out = new ByteArrayOutputStream();
72+
IOUtils.copy(raw, out);
73+
out.flush();
74+
return out.toByteArray();
75+
}
76+
}
77+
7278
private static byte[] readResource(final Resource p) throws IOException {
73-
try (InputStream in = p.getResourceLoader().getResourceStream(p.getName());
79+
try (Reader in =
80+
p.getResourceLoader().getResourceReader(p.getName(), StandardCharsets.UTF_8.name());
7481
ByteArrayOutputStream byteOut = new ByteArrayOutputStream()) {
7582
IOUtils.copy(in, byteOut);
7683
return byteOut.toByteArray();
7784
}
7885
}
7986

87+
private byte[] compress(PluginEntry pluginEntry) throws IOException {
88+
try (InputStream raw = plugin.getContentScanner().getInputStream(pluginEntry)) {
89+
final ByteArrayOutputStream out = new ByteArrayOutputStream();
90+
final GZIPOutputStream gz = new GZIPOutputStream(out);
91+
IOUtils.copy(raw, gz);
92+
gz.finish();
93+
gz.flush();
94+
return out.toByteArray();
95+
}
96+
}
97+
8098
private static byte[] compress(final byte[] raw) throws IOException {
8199
final ByteArrayOutputStream out = new ByteArrayOutputStream();
82100
final GZIPOutputStream gz = new GZIPOutputStream(out);
@@ -87,16 +105,22 @@ private static byte[] compress(final byte[] raw) throws IOException {
87105
}
88106

89107
private final RuntimeInstance velocity;
108+
private final SiteStaticDirectoryServlet siteStaticServlet;
109+
private final Plugin plugin;
90110

91111
@Inject
92112
VelocityStaticServlet(
93-
@Named("PluginRuntimeInstance") final Provider<RuntimeInstance> velocityRuntimeProvider) {
113+
@Named("PluginRuntimeInstance") final Provider<RuntimeInstance> velocityRuntimeProvider,
114+
SiteStaticDirectoryServlet siteStaticServlet,
115+
Plugin plugin) {
94116
this.velocity = velocityRuntimeProvider.get();
117+
this.siteStaticServlet = siteStaticServlet;
118+
this.plugin = plugin;
95119
}
96120

97121
private Resource local(final HttpServletRequest req) {
98122
final String name = req.getPathInfo();
99-
if (name.length() < 2 || !name.startsWith("/") || isUnreasonableName(name)) {
123+
if (name.length() < 2 || !name.startsWith("/")) {
100124
// Too short to be a valid file name, or doesn't start with
101125
// the path info separator like we expected.
102126
//
@@ -112,6 +136,24 @@ private Resource local(final HttpServletRequest req) {
112136
}
113137
}
114138

139+
private boolean isVelocityStaticResource(String resourceName) {
140+
final int dot = resourceName.lastIndexOf('.');
141+
final String ext = 0 < dot ? resourceName.substring(dot + 1) : "";
142+
return VELOCITY_STATIC_TYPES.contains(ext.toLowerCase());
143+
}
144+
145+
private String resourceName(HttpServletRequest req) {
146+
final String name = req.getPathInfo();
147+
if (name.length() < 2 || !name.startsWith("/") || isUnreasonableName(name)) {
148+
// Too short to be a valid file name, or doesn't start with
149+
// the path info separator like we expected.
150+
//
151+
return null;
152+
}
153+
154+
return name.substring(1);
155+
}
156+
115157
private static boolean isUnreasonableName(String name) {
116158
if (name.charAt(name.length() - 1) == '/') return true; // no suffix
117159
if (name.indexOf('\\') >= 0) return true; // no windows/dos stlye paths
@@ -131,29 +173,68 @@ protected long getLastModified(final HttpServletRequest req) {
131173

132174
@Override
133175
protected void doGet(final HttpServletRequest req, final HttpServletResponse rsp)
134-
throws IOException {
135-
final Resource p = local(req);
136-
if (p == null) {
176+
throws IOException, ServletException {
177+
String resourceName = resourceName(req);
178+
if (isUnreasonableName(resourceName) || !resourceName.startsWith(STATIC_PATH_PREFIX)) {
137179
CacheHeaders.setNotCacheable(rsp);
138180
rsp.setStatus(HttpServletResponse.SC_NOT_FOUND);
139181
return;
140182
}
141183

142-
final String type = contentType(p.getName());
143-
final byte[] tosend;
144-
if (!type.equals("application/x-javascript") && RequestUtil.acceptsGzipEncoding(req)) {
145-
rsp.setHeader("Content-Encoding", "gzip");
146-
tosend = compress(readResource(p));
147-
} else {
148-
tosend = readResource(p);
184+
if (isVelocityStaticResource(resourceName)) {
185+
final Resource p = local(req);
186+
if (p == null) {
187+
CacheHeaders.setNotCacheable(rsp);
188+
rsp.setStatus(HttpServletResponse.SC_NOT_FOUND);
189+
return;
190+
}
191+
192+
final String type = contentType(p.getName());
193+
final byte[] tosend;
194+
if (!type.equals("application/x-javascript") && RequestUtil.acceptsGzipEncoding(req)) {
195+
rsp.setHeader("Content-Encoding", "gzip");
196+
tosend = compress(readResource(p));
197+
} else {
198+
tosend = readResource(p);
199+
}
200+
201+
CacheHeaders.setCacheable(req, rsp, 12, TimeUnit.HOURS);
202+
rsp.setDateHeader("Last-Modified", p.getLastModified());
203+
rsp.setContentType(type);
204+
rsp.setContentLength(tosend.length);
205+
try (OutputStream out = rsp.getOutputStream()) {
206+
out.write(tosend);
207+
}
149208
}
150209

151-
CacheHeaders.setCacheable(req, rsp, 12, TimeUnit.HOURS);
152-
rsp.setDateHeader("Last-Modified", p.getLastModified());
153-
rsp.setContentType(type);
154-
rsp.setContentLength(tosend.length);
155-
try (OutputStream out = rsp.getOutputStream()) {
156-
out.write(tosend);
210+
Optional<PluginEntry> jarResource = plugin.getContentScanner().getEntry(resourceName);
211+
if (jarResource.isPresent()) {
212+
final String type = contentType(resourceName);
213+
final byte[] tosend;
214+
if (!type.equals("application/x-javascript") && RequestUtil.acceptsGzipEncoding(req)) {
215+
rsp.setHeader("Content-Encoding", "gzip");
216+
tosend = compress(jarResource.get());
217+
} else {
218+
tosend = read(jarResource.get());
219+
}
220+
221+
CacheHeaders.setCacheable(req, rsp, 12, TimeUnit.HOURS);
222+
rsp.setContentType(type);
223+
rsp.setContentLength(tosend.length);
224+
try (OutputStream out = rsp.getOutputStream()) {
225+
out.write(tosend);
226+
}
227+
} else {
228+
229+
HttpServletRequestWrapper mappedReq =
230+
new HttpServletRequestWrapper(req) {
231+
232+
@Override
233+
public String getPathInfo() {
234+
return super.getPathInfo().substring(STATIC_PATH_PREFIX.length());
235+
}
236+
};
237+
siteStaticServlet.service(mappedReq, rsp);
157238
}
158239
}
159240
}

github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
import javax.servlet.ServletException;
5252
import javax.servlet.http.HttpServletRequest;
5353
import javax.servlet.http.HttpServletResponse;
54-
import org.apache.commons.lang.StringUtils;
54+
import org.apache.commons.lang3.StringUtils;
5555
import org.eclipse.jgit.errors.ConfigInvalidException;
5656
import org.kohsuke.github.GHKey;
5757
import org.kohsuke.github.GHMyself;

github-plugin/src/main/resources/static/scope.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ <h5>Which level of GitHub access do you need?</h5>
5555
#set ( $scopeItems = $config.scopes.get($scope) )
5656
#foreach ( $scopeItem in $scopeItems )
5757
$scopeItem.description
58-
#if ( $velocityCount < $scopeItems.size())
58+
#if ( $foreach.count < $scopeItems.size())
5959
,&nbsp;
6060
#end
6161
#end

0 commit comments

Comments
 (0)