From ad64ca831a212b3c8cd9a10684512d85c5669c29 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 21 Oct 2013 12:16:04 +1100 Subject: [PATCH 1/2] 419846 JDBCSessionManager doesn't determine dirty state correctly --- .../org/eclipse/jetty/server/session/JDBCSessionManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java index 99314557ee3..2552055c9e4 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java @@ -244,7 +244,7 @@ public class JDBCSessionManager extends AbstractSessionManager @Override public void setAttribute (String name, Object value) { - _dirty=updateAttribute(name, value); + _dirty = (updateAttribute(name, value) || _dirty); } @Override From 24c1b30495551a1d4ab86052af60b54159c04f06 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 21 Oct 2013 12:19:16 +1100 Subject: [PATCH 2/2] 418732 - Add whiteListByPath mode to IPAccessHandler Fixed the contribution. The PathMap changes were not correct as "" is the pattern for "/" path only. Also removed the use of the lazy list --- .../java/org/eclipse/jetty/http/PathMap.java | 47 ++++++++--------- .../org/eclipse/jetty/http/PathMapTest.java | 5 +- .../jetty/server/handler/IPAccessHandler.java | 51 +++++++++---------- .../server/handler/IPAccessHandlerTest.java | 7 +-- 4 files changed, 52 insertions(+), 58 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/PathMap.java b/jetty-http/src/main/java/org/eclipse/jetty/http/PathMap.java index 02175fd1027..00b77a5a168 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/PathMap.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/PathMap.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.http; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -39,6 +40,7 @@ import org.eclipse.jetty.util.URIUtil; * /foo/* - a prefix path specification (must end '/*'). * *.ext - a suffix path specification. * / - the default path specification. + * "" - the / path specification * * Matching is performed in the following order *
  • Exact match. @@ -134,7 +136,6 @@ public class PathMap extends HashMap MappedEntry entry = new MappedEntry<>("",object); entry.setMapped(""); _exactMap.put("", entry); - _prefixDefault = entry; return super.put("", object); } @@ -268,20 +269,22 @@ public class PathMap extends HashMap /** Get all entries matched by the path. * Best match first. * @param path Path to match - * @return LazyList of Map.Entry instances key=pathSpec + * @return List of Map.Entry instances key=pathSpec */ - public Object getLazyMatches(String path) + public List> getMatches(String path) { MappedEntry entry; - Object entries=null; + List> entries=new ArrayList<>(); if (path==null) - return LazyList.getList(entries); + return entries; + if (path.length()==0) + return _defaultSingletonList; // try exact match entry=_exactMap.get(path); if (entry!=null) - entries=LazyList.add(entries,entry); + entries.add(entry); // prefix search int l=path.length(); @@ -294,14 +297,14 @@ public class PathMap extends HashMap break; String key = entry.getKey(); if (key.length()-2>=path.length() || path.charAt(key.length()-2)=='/') - entries=LazyList.add(entries,entry); + entries.add(entry); i=key.length()-3; } // Prefix Default if (_prefixDefault!=null) - entries=LazyList.add(entries,_prefixDefault); + entries.add(_prefixDefault); // Extension search i=0; @@ -310,32 +313,24 @@ public class PathMap extends HashMap { entry=suffix_map.get(path,i+1,l-i-1); if (entry!=null) - entries=LazyList.add(entries,entry); + entries.add(entry); } + // root match + if ("/".equals(path)) + { + entry=_exactMap.get(""); + if (entry!=null) + entries.add(entry); + } + // Default if (_default!=null) - { - // Optimization for just the default - if (entries==null) - return _defaultSingletonList; - - entries=LazyList.add(entries,_default); - } + entries.add(_default); return entries; } - /* --------------------------------------------------------------- */ - /** Get all entries matched by the path. - * Best match first. - * @param path Path to match - * @return List of Map.Entry instances key=pathSpec - */ - public List> getMatches(String path) - { - return LazyList.getList(getLazyMatches(path)); - } /* --------------------------------------------------------------- */ /** Return whether the path matches any entries in the PathMap, diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/PathMapTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/PathMapTest.java index 65481f14351..f400ad26e97 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/PathMapTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/PathMapTest.java @@ -65,6 +65,7 @@ public class PathMapTest { "/animal/path.gz", "5"}, { "/Other/path", "8"}, { "/\u20ACuro/path", "11"}, + { "/", "10"}, }; for (String[] test : tests) @@ -78,8 +79,8 @@ public class PathMapTest p.getMatches("/animal/bird/path.tar.gz").toString()); assertEquals("Dir matches", "[/animal/fish/*=4, /animal/*=5, /=8]", p.getMatches("/animal/fish/").toString()); assertEquals("Dir matches", "[/animal/fish/*=4, /animal/*=5, /=8]", p.getMatches("/animal/fish").toString()); - assertEquals("Dir matches", "[/=8]", p.getMatches("/").toString()); - assertEquals("Dir matches", "[=10, /=8]", p.getMatches("").toString()); + assertEquals("Root matches", "[=10, /=8]",p.getMatches("/").toString()); + assertEquals("Dir matches", "[/=8]", p.getMatches("").toString()); assertEquals("pathMatch exact", "/Foo/bar", PathMap.pathMatch("/Foo/bar", "/Foo/bar")); assertEquals("pathMatch prefix", "/Foo", PathMap.pathMatch("/Foo/*", "/Foo/bar")); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/IPAccessHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/IPAccessHandler.java index acb3c8b31eb..03cf516788c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/IPAccessHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/IPAccessHandler.java @@ -20,8 +20,6 @@ package org.eclipse.jetty.server.handler; import java.io.IOException; import java.net.InetSocketAddress; -import java.util.Collections; -import java.util.List; import java.util.Map; import javax.servlet.ServletException; @@ -203,7 +201,7 @@ public class IPAccessHandler extends HandlerWrapper public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { // Get the real remote IP (not the one set by the forwarded headers (which may be forged)) - HttpChannel channel = baseRequest.getHttpChannel(); + HttpChannel channel = baseRequest.getHttpChannel(); if (channel!=null) { EndPoint endp=channel.getEndPoint(); @@ -307,40 +305,38 @@ public class IPAccessHandler extends HandlerWrapper boolean match = false; boolean matchedByPath = false; - Object whiteObj = _white.getLazyMatches(path); - if (whiteObj != null) + for (Map.Entry> entry : _white.getMatches(path)) { - matchedByPath = true; - List whiteList = (whiteObj instanceof List) ? (List)whiteObj : Collections.singletonList(whiteObj); - - for (Object entry: whiteList) + matchedByPath=true; + IPAddressMap addrMap = entry.getValue(); + if ((addrMap!=null && (addrMap.size()==0 || addrMap.match(addr)!=null))) { - IPAddressMap addrMap = ((Map.Entry>)entry).getValue(); - if (match = (addrMap!=null && (addrMap.size()==0 || addrMap.match(addr)!=null))) - break; + match=true; + break; } } - - if (!_whiteListByPath && !match) // Default behaviour - return false; - else if (_whiteListByPath && matchedByPath && !match) // Fail if only matched by path - return false; + + if (_whiteListByPath) + { + if (matchedByPath && !match) + return false; + } + else + { + if (!match) + return false; + } } if (_black.size() > 0) { - Object blackObj = _black.getLazyMatches(path); - if (blackObj != null) + for (Map.Entry> entry : _black.getMatches(path)) { - List blackList = (blackObj instanceof List) ? (List)blackObj : Collections.singletonList(blackObj); - - for (Object entry: blackList) - { - IPAddressMap addrMap = ((Map.Entry>)entry).getValue(); - if (addrMap!=null && (addrMap.size()==0 || addrMap.match(addr)!=null)) - return false; - } + IPAddressMap addrMap = entry.getValue(); + if (addrMap!=null && (addrMap.size()==0 || addrMap.match(addr)!=null)) + return false; } + } return true; @@ -350,6 +346,7 @@ public class IPAccessHandler extends HandlerWrapper /** * Dump the handler configuration */ + @Override public String dump() { StringBuilder buf = new StringBuilder(); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/IPAccessHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/IPAccessHandlerTest.java index 5a8c5e6f5d7..6b8d90baf08 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/IPAccessHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/IPAccessHandlerTest.java @@ -77,6 +77,7 @@ public class IPAccessHandlerTest _handler = new IPAccessHandler(); _handler.setHandler(new AbstractHandler() { + @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { baseRequest.setHandled(true); @@ -283,7 +284,7 @@ public class IPAccessHandlerTest {"127.0.0.1|/dump/info;127.0.0.1|/dump/test", "", "127.0.0.1", "/dump/fail", "403", false}, {"127.0.0.0-2|", "", "127.0.0.1", "/", "200", false}, - {"127.0.0.0-2|", "", "127.0.0.1", "/dump/info", "200", false}, + {"127.0.0.0-2|", "", "127.0.0.1", "/dump/info", "403", false}, {"127.0.0.0-2|/", "", "127.0.0.1", "/", "200", false}, {"127.0.0.0-2|/", "", "127.0.0.1", "/dispatch", "403", false}, @@ -334,7 +335,7 @@ public class IPAccessHandlerTest {"", "127.0.0.1|/dump/info;127.0.0.1|/dump/test", "127.0.0.1", "/dump/fail", "200", false}, {"", "127.0.0.0-2|", "127.0.0.1", "/", "403", false}, - {"", "127.0.0.0-2|", "127.0.0.1", "/dump/info", "403", false}, + {"", "127.0.0.0-2|", "127.0.0.1", "/dump/info", "200", false}, {"", "127.0.0.0-2|/", "127.0.0.1", "/", "403", false}, {"", "127.0.0.0-2|/", "127.0.0.1", "/dispatch", "200", false}, @@ -490,7 +491,7 @@ public class IPAccessHandlerTest {"", "127.0.0.1|/dump/info;127.0.0.1|/dump/test", "127.0.0.1", "/dump/fail", "200", true}, {"", "127.0.0.0-2|", "127.0.0.1", "/", "403", true}, - {"", "127.0.0.0-2|", "127.0.0.1", "/dump/info", "403", true}, + {"", "127.0.0.0-2|", "127.0.0.1", "/dump/info", "200", true}, {"", "127.0.0.0-2|/", "127.0.0.1", "/", "403", true}, {"", "127.0.0.0-2|/", "127.0.0.1", "/dispatch", "200", true},