From 7016504ecc1c7eeda721ff1c34f4af0684cd9c2e Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 7 Jun 2012 11:41:25 -0700 Subject: [PATCH 1/3] Add simple tests for RegexPipeline Change-Id: Ie800c55702ea9724b393be0a8b36e0e4da1a6e0d --- .../jgit/http/test/RegexPipelineTest.java | 209 ++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RegexPipelineTest.java diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RegexPipelineTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RegexPipelineTest.java new file mode 100644 index 000000000..64fbc01d4 --- /dev/null +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RegexPipelineTest.java @@ -0,0 +1,209 @@ +/* + * Copyright (C) 2012, Google Inc. + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.http.test; + +import static org.junit.Assert.assertEquals; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.PrintWriter; +import java.net.HttpURLConnection; +import java.net.URI; + +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jgit.http.server.glue.MetaServlet; +import org.eclipse.jgit.http.server.glue.RegexGroupFilter; +import org.eclipse.jgit.junit.http.AppServer; +import org.eclipse.jgit.junit.http.HttpTestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class RegexPipelineTest extends HttpTestCase { + private ServletContextHandler ctx; + + private static class Servlet extends HttpServlet { + private static final long serialVersionUID = 1L; + + private final String name; + + private Servlet(String name) { + this.name = name; + } + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse res) + throws IOException { + res.setStatus(200); + PrintWriter out = new PrintWriter(res.getOutputStream()); + out.write(name); + out.write("\n"); + out.write(String.valueOf(req.getServletPath())); + out.write("\n"); + out.write(String.valueOf(req.getPathInfo())); + out.write("\n"); + out.flush(); + } + } + + @Before + public void setUp() throws Exception { + server = new AppServer(); + ctx = server.addContext("/"); + } + + @After + public void tearDown() throws Exception { + server.tearDown(); + } + + @Test + public void testSimpleRegex() throws Exception { + MetaServlet s = new MetaServlet(); + s.serveRegex("^(/a|/b)$").with(new Servlet("test")); + ctx.addServlet(new ServletHolder(s), "/*"); + server.setUp(); + + final URI uri = server.getURI(); + HttpURLConnection c; + BufferedReader r; + + c = ((HttpURLConnection) uri.resolve("/a").toURL() + .openConnection()); + assertEquals(200, c.getResponseCode()); + r = new BufferedReader(new InputStreamReader(c.getInputStream())); + assertEquals("test", r.readLine()); + assertEquals("", r.readLine()); + assertEquals("/a", r.readLine()); + assertEquals(null, r.readLine()); + + c = ((HttpURLConnection) uri.resolve("/b").toURL() + .openConnection()); + assertEquals(200, c.getResponseCode()); + r = new BufferedReader(new InputStreamReader(c.getInputStream())); + assertEquals("test", r.readLine()); + assertEquals("", r.readLine()); + assertEquals("/b", r.readLine()); + assertEquals(null, r.readLine()); + + c = ((HttpURLConnection) uri.resolve("/c").toURL() + .openConnection()); + assertEquals(404, c.getResponseCode()); + } + + @Test + public void testServeOrdering() throws Exception { + MetaServlet s = new MetaServlet(); + s.serveRegex("^(/a)$").with(new Servlet("test1")); + s.serveRegex("^(/a+)$").with(new Servlet("test2")); + ctx.addServlet(new ServletHolder(s), "/*"); + server.setUp(); + + final URI uri = server.getURI(); + HttpURLConnection c; + BufferedReader r; + + c = ((HttpURLConnection) uri.resolve("/a").toURL() + .openConnection()); + assertEquals(200, c.getResponseCode()); + r = new BufferedReader(new InputStreamReader(c.getInputStream())); + assertEquals("test1", r.readLine()); + assertEquals("", r.readLine()); + assertEquals("/a", r.readLine()); + assertEquals(null, r.readLine()); + } + + @Test + public void testRegexGroupFilter() throws Exception { + MetaServlet s = new MetaServlet(); + s.serveRegex("^(/a)(/b)$") + .with(new Servlet("test1")); + s.serveRegex("^(/c)(/d)$") + .through(new RegexGroupFilter(1)) + .with(new Servlet("test2")); + s.serveRegex("^(/e)/f(/g)$") + .through(new RegexGroupFilter(2)) + .with(new Servlet("test3")); + ctx.addServlet(new ServletHolder(s), "/*"); + server.setUp(); + + final URI uri = server.getURI(); + HttpURLConnection c; + BufferedReader r; + + c = ((HttpURLConnection) uri.resolve("/a/b").toURL() + .openConnection()); + assertEquals(200, c.getResponseCode()); + r = new BufferedReader(new InputStreamReader(c.getInputStream())); + assertEquals("test1", r.readLine()); + assertEquals("", r.readLine()); + // No RegexGroupFilter defaults to first group. + assertEquals("/a", r.readLine()); + assertEquals(null, r.readLine()); + + c = ((HttpURLConnection) uri.resolve("/c/d").toURL() + .openConnection()); + assertEquals(200, c.getResponseCode()); + r = new BufferedReader(new InputStreamReader(c.getInputStream())); + assertEquals("test2", r.readLine()); + assertEquals("", r.readLine()); + assertEquals("/c", r.readLine()); + assertEquals(null, r.readLine()); + + c = ((HttpURLConnection) uri.resolve("/e/f/g").toURL() + .openConnection()); + assertEquals(200, c.getResponseCode()); + r = new BufferedReader(new InputStreamReader(c.getInputStream())); + assertEquals("test3", r.readLine()); + assertEquals("/e/f", r.readLine()); + assertEquals("/g", r.readLine()); + assertEquals(null, r.readLine()); + } +} From 61c4e39067777633ea5d6d02f7493b4c459aef20 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 7 Jun 2012 12:08:48 -0700 Subject: [PATCH 2/3] Expand RegexPipeline documentation Include some behaviors that were not clear to me until I had used it a few times. Warn about broken behavior for capture groups that do not match. It would be nice to support these, but even for the cases where it's clear what the behavior should be, it would be infeasible to implement. For example, consider the second group of the regex "(/a)/b(/c)?" matched against the path "/a/b". We might want getServletPath() to return "/a/b" and getPathInfo() to return null, but this is hard to implement: there's no easy way to say "the substring up to the point where (/c) would have matched if it were in the string even though it's not." And even if we could, it's not clear there is even a right answer in the general case. Moreover, ideally we could warn about such broken patterns at servlet initialization time, rather than at runtime, but even answering the question of whether there are capture groups that might not match requires more customized regular expression parsing than we want to embark on. Hence, the best we can do is document how it fails. Change-Id: I7bd5011f5bd387f9345a0e79b22a4d7ed918a190 --- .../jgit/http/server/glue/RegexPipeline.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java index a4acd1550..384ff45a3 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java @@ -64,13 +64,20 @@ import javax.servlet.http.HttpServletResponse; *

* If there are capture groups in the regular expression, the matched ranges of * the capture groups are stored as an array of modified HttpServetRequests, - * into the request attribute {@link MetaFilter#REGEX_GROUPS}. + * into the request attribute {@link MetaFilter#REGEX_GROUPS}. Using a capture + * group that may not capture, e.g. {@code "(/foo)?"}, will cause an error at + * request handling time. *

- * Each servlet request has been altered to have its {@code getPathInfo()} - * method return the matched text of the corresponding capture group. A - * {@link RegexGroupFilter} can be applied in the pipeline to switch the current - * HttpServletRequest to reference a different capture group before running - * additional filters, or the final servlet. + * Each servlet request has been altered to have its {@code getServletPath()} + * method return the original path info up to the beginning of the corresponding + * capture group, and its {@code getPathInfo()} method return the matched text. + * A {@link RegexGroupFilter} can be applied in the pipeline to switch the + * current HttpServletRequest to reference a different capture group before + * running additional filters, or the final servlet. + *

+ * Note that for {@code getPathInfo()} to start with a leading "/" as described + * in the servlet documentation, capture groups must actually capture the + * leading "/". *

* This class dispatches the remainder of the pipeline using the first capture * group as the current request, making {@code RegexGroupFilter} required only From b12f80e71ca2eee6ac121a8e8a3d271408ca3982 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Fri, 8 Jun 2012 12:13:23 -0700 Subject: [PATCH 3/3] Add MetaFilter.serveRegex(Pattern) This allows the use of precompiled patterns, such as those compiled with flags. Change-Id: I1c87fea98e246004aecbae3aabaf1d21fbf3176e --- .../eclipse/jgit/http/server/glue/MetaFilter.java | 12 ++++++++++++ .../eclipse/jgit/http/server/glue/RegexPipeline.java | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/MetaFilter.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/MetaFilter.java index 8cb3bea1f..2187cfaa4 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/MetaFilter.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/MetaFilter.java @@ -52,6 +52,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Pattern; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -116,6 +117,17 @@ public class MetaFilter implements Filter { return register(new RegexPipeline.Binder(expression)); } + /** + * Construct a binding for a regular expression. + * + * @param pattern + * the regular expression to pattern match the URL against. + * @return binder for the passed expression. + */ + public ServletBinder serveRegex(Pattern pattern) { + return register(new RegexPipeline.Binder(pattern)); + } + public void init(FilterConfig filterConfig) throws ServletException { servletContext = filterConfig.getServletContext(); } diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java index 384ff45a3..2ef71368d 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java @@ -91,6 +91,10 @@ class RegexPipeline extends UrlPipeline { pattern = Pattern.compile(p); } + Binder(final Pattern p) { + pattern = p; + } + UrlPipeline create() { return new RegexPipeline(pattern, getFilters(), getServlet()); }