From 32ec8255db2b2923142dbe0f8b26128ca26d79a9 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 27 Jun 2013 10:19:51 +1000 Subject: [PATCH] 411538 Use Replacement character for bad parameter % encodings --- .../org/eclipse/jetty/server/HttpURITest.java | 31 +++------ .../org/eclipse/jetty/server/RequestTest.java | 26 ++------ .../java/org/eclipse/jetty/util/TypeUtil.java | 2 +- .../org/eclipse/jetty/util/UrlEncoded.java | 64 +++++++++++++------ .../eclipse/jetty/util/Utf8Appendable.java | 1 + 5 files changed, 62 insertions(+), 62 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpURITest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpURITest.java index a770f95c786..05958ccd3c6 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpURITest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpURITest.java @@ -33,6 +33,7 @@ import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.TypeUtil; +import org.eclipse.jetty.util.Utf8Appendable; import org.junit.Assert; import org.junit.Test; @@ -322,29 +323,15 @@ public class HttpURITest { } - try - { - HttpURI huri=new HttpURI(uri); - MultiMap params = new MultiMap<>(); - huri.decodeQueryTo(params); - System.err.println(params); - Assert.assertTrue(false); - } - catch (IllegalArgumentException e) - { - } + HttpURI huri=new HttpURI(uri); + MultiMap params = new MultiMap<>(); + huri.decodeQueryTo(params); + assertEquals("data"+Utf8Appendable.REPLACEMENT+"here"+Utf8Appendable.REPLACEMENT,params.getValue("invalid",0)); - try - { - HttpURI huri=new HttpURI(uri); - MultiMap params = new MultiMap<>(); - huri.decodeQueryTo(params,"UTF-8"); - System.err.println(params); - Assert.assertTrue(false); - } - catch (IllegalArgumentException e) - { - } + huri=new HttpURI(uri); + params = new MultiMap<>(); + huri.decodeQueryTo(params,"UTF-8"); + assertEquals("data"+Utf8Appendable.REPLACEMENT+"here"+Utf8Appendable.REPLACEMENT,params.getValue("invalid",0)); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 3249d964b82..068a8df7e8b 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -52,6 +52,7 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.MultiPartInputStreamParser; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.Utf8Appendable; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.log.StdErrLog; @@ -100,24 +101,11 @@ public class RequestTest @Override public boolean check(HttpServletRequest request,HttpServletResponse response) { - Map map = null; - try - { - //do the parse - request.getParameterMap(); - Assert.fail("Expected parsing failure"); - return false; - } - catch (Exception e) - { - //catch the error and check the param map is not null - map = request.getParameterMap(); - assertFalse(map == null); - assertTrue(map.isEmpty()); - - Enumeration names = request.getParameterNames(); - assertFalse(names.hasMoreElements()); - } + Map map = null; + //do the parse + map = request.getParameterMap(); + assertEquals("aaa"+Utf8Appendable.REPLACEMENT+"bbb",map.get("param")[0]); + assertEquals("value",map.get("other")[0]); return true; } @@ -125,7 +113,7 @@ public class RequestTest //Send a request with query string with illegal hex code to cause //an exception parsing the params - String request="GET /?param=%ZZaaa HTTP/1.1\r\n"+ + String request="GET /?param=aaa%ZZbbb&other=value HTTP/1.1\r\n"+ "Host: whatever\r\n"+ "Content-Type: text/html;charset=utf8\n"+ "Connection: close\n"+ diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index 68de40490a5..d63fe379c47 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -353,7 +353,7 @@ public class TypeUtil { byte b = (byte)((c & 0x1f) + ((c >> 6) * 0x19) - 0x10); if (b<0 || b>15) - throw new IllegalArgumentException("!hex "+c); + throw new NumberFormatException("!hex "+c); return b; } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java b/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java index fed74d5ed43..dfe02033856 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java @@ -323,7 +323,13 @@ public class UrlEncoded extends MultiMap implements Cloneable { i++; if (i+4 implements Cloneable } } else - buffer.append((byte)((convertHexDigit(raw[++i])<<4) + convertHexDigit(raw[++i]))); + { + byte hi=raw[++i]; + byte lo=raw[++i]; + buffer.append((byte)((convertHexDigit(hi)<<4) + convertHexDigit(lo))); + } } else { @@ -350,6 +360,12 @@ public class UrlEncoded extends MultiMap implements Cloneable LOG.warn(e.toString()); LOG.debug(e); } + catch(NumberFormatException e) + { + buffer.append(Utf8Appendable.REPLACEMENT_UTF8,0,3); + LOG.warn(e.toString()); + LOG.debug(e); + } } if (key != null) @@ -552,6 +568,12 @@ public class UrlEncoded extends MultiMap implements Cloneable LOG.warn(e.toString()); LOG.debug(e); } + catch(NumberFormatException e) + { + buffer.append(Utf8Appendable.REPLACEMENT_UTF8,0,3); + LOG.warn(e.toString()); + LOG.debug(e); + } if (maxLength>=0 && (++totalLength > maxLength)) throw new IllegalStateException("Form too large"); } @@ -798,9 +820,10 @@ public class UrlEncoded extends MultiMap implements Cloneable LOG.warn(e.toString()); LOG.debug(e); } - catch(NumberFormatException nfe) + catch(NumberFormatException e) { - LOG.debug(nfe); + LOG.warn(e.toString()); + LOG.debug(e); buffer.getStringBuffer().append(Utf8Appendable.REPLACEMENT); } } @@ -870,32 +893,33 @@ public class UrlEncoded extends MultiMap implements Cloneable { if ('u'==encoded.charAt(offset+i+1)) { - if (i+6