Author: sergeyb Date: Mon Mar 4 21:23:44 2013 New Revision: 1452534 URL: http://svn.apache.org/r1452534 Log: Merged revisions 1452337 via svnmerge from https://svn.apache.org/repos/asf/cxf/branches/2.7.x-fixes
................ r1452337 | sergeyb | 2013-03-04 14:59:36 +0000 (Mon, 04 Mar 2013) | 9 lines Merged revisions 1452335 via svnmerge from https://svn.apache.org/repos/asf/cxf/trunk ........ r1452335 | sergeyb | 2013-03-04 14:53:13 +0000 (Mon, 04 Mar 2013) | 1 line [CXF-4851] Improving the way UriBuilder handles encoding in case of build() calls ........ ................ Modified: cxf/branches/2.6.x-fixes/ (props changed) cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriInfoImpl.java cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriBuilderImplTest.java cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/model/URITemplateTest.java Propchange: cxf/branches/2.6.x-fixes/ ------------------------------------------------------------------------------ --- svn:mergeinfo (added) +++ svn:mergeinfo Mon Mar 4 21:23:44 2013 @@ -0,0 +1,2 @@ +/cxf/branches/2.7.x-fixes:1452337 +/cxf/trunk:1452335 Propchange: cxf/branches/2.6.x-fixes/ ------------------------------------------------------------------------------ Binary property 'svnmerge-integrated' - no diff available. Modified: cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java URL: http://svn.apache.org/viewvc/cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java?rev=1452534&r1=1452533&r2=1452534&view=diff ============================================================================== --- cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java (original) +++ cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java Mon Mar 4 21:23:44 2013 @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -77,28 +78,38 @@ public class UriBuilderImpl extends UriB } private URI doBuild(boolean fromEncoded, Object... values) { + if (values == null) { + throw new IllegalArgumentException("Template parameter values are set to null"); + } + for (int i = 0; i < values.length; i++) { + if (values[i] == null) { + throw new IllegalArgumentException("Template parameter value is set to null"); + } + } - String thePath = buildPath(fromEncoded); + String thePath = buildPath(); URITemplate pathTempl = new URITemplate(thePath); - thePath = substituteVarargs(pathTempl, values, 0); + thePath = substituteVarargs(pathTempl, values, 0, false, fromEncoded); - String theQuery = buildQuery(fromEncoded); + String theQuery = buildQuery(); int queryTemplateVarsSize = 0; if (theQuery != null) { URITemplate queryTempl = new URITemplate(theQuery); - int lengthDiff = values.length - pathTempl.getVariables().size(); - if (lengthDiff > 0) { + queryTemplateVarsSize = queryTempl.getVariables().size(); + if (queryTemplateVarsSize > 0) { + int lengthDiff = values.length - pathTempl.getVariables().size(); queryTemplateVarsSize = queryTempl.getVariables().size(); - theQuery = substituteVarargs(queryTempl, values, values.length - lengthDiff); + theQuery = substituteVarargs(queryTempl, values, values.length - lengthDiff, true, fromEncoded); } } String theFragment = fragment; if (theFragment != null) { URITemplate fragmentTempl = new URITemplate(theFragment); - int lengthDiff = values.length - pathTempl.getVariables().size() - queryTemplateVarsSize; - if (lengthDiff > 0) { - theFragment = substituteVarargs(fragmentTempl, values, values.length - lengthDiff); + if (fragmentTempl.getVariables().size() > 0) { + int lengthDiff = values.length - pathTempl.getVariables().size() - queryTemplateVarsSize; + theFragment = substituteVarargs(fragmentTempl, values, + values.length - lengthDiff, true, fromEncoded); } } @@ -111,19 +122,17 @@ public class UriBuilderImpl extends UriB private URI buildURI(boolean fromEncoded, String thePath, String theQuery, String theFragment) throws URISyntaxException { - if (fromEncoded) { + if (fromEncoded) { return buildURIFromEncoded(thePath, theQuery, theFragment); } else if (!isSchemeOpaque()) { if ((scheme != null || host != null || userInfo != null) && thePath.length() != 0 && !thePath.startsWith("/")) { thePath = "/" + thePath; } - if (theQuery != null && HttpUtils.isPartiallyEncoded(theQuery)) { - try { - return buildURIFromEncoded(thePath, theQuery, theFragment); - } catch (Exception ex) { - // lets try the option below - } + try { + return buildURIFromEncoded(thePath, theQuery, theFragment); + } catch (Exception ex) { + // lets try the option below } URI uri = new URI(scheme, userInfo, host, port, thePath, theQuery, theFragment); @@ -177,45 +186,8 @@ public class UriBuilderImpl extends UriB return schemeSpecificPart != null; } - private String substituteVarargs(URITemplate templ, Object[] values, int ind) { - Map<String, String> varValueMap = new HashMap<String, String>(); - - // vars in set are properly ordered due to linking in hash set - Set<String> uniqueVars = new LinkedHashSet<String>(templ.getVariables()); - if (values.length < uniqueVars.size()) { - throw new IllegalArgumentException("Unresolved variables; only " + values.length - + " value(s) given for " + uniqueVars.size() - + " unique variable(s)"); - } - int idx = ind; - for (String var : uniqueVars) { - Object oval = values[idx++]; - if (oval == null) { - throw new IllegalArgumentException("No object for " + var); - } - varValueMap.put(var, oval.toString()); - } - return templ.substitute(varValueMap); - } - @Override public URI buildFromEncoded(Object... values) throws IllegalArgumentException, UriBuilderException { - // Problem: multi-arg URI c-tor always forces encoding, operation contract would be broken; - // use os single-arg URI c-tor requires unnecessary concatenate-parse roundtrip. - // While decoding back given values and passing as non-decoded to regular build() method - // is promising unfortunatley it causes the loss of encoded reserved values such as +, - // which might cause problems if consumers do rely on URLEncoder which would turn '+' into - // ' ' or would break the contract in when query parameters are expected to have %2B - if (values == null) { - throw new IllegalArgumentException("Template parameter values are set to null"); - } - for (int i = 0; i < values.length; i++) { - if (values[i] == null) { - throw new IllegalArgumentException("Template parameter value is set to null"); - } - - values[i] = HttpUtils.encodePartiallyEncoded(values[i].toString(), false); - } return doBuild(true, values); } @@ -228,23 +200,59 @@ public class UriBuilderImpl extends UriB private URI doBuildFromMap(Map<String, ? extends Object> map, boolean fromEncoded) throws IllegalArgumentException, UriBuilderException { try { - String thePath = buildPath(fromEncoded); - thePath = substituteMapped(thePath, map); + String thePath = buildPath(); + thePath = substituteMapped(thePath, map, false, fromEncoded); - String theQuery = buildQuery(fromEncoded); + String theQuery = buildQuery(); if (theQuery != null) { - theQuery = substituteMapped(theQuery, map); + theQuery = substituteMapped(theQuery, map, true, fromEncoded); } - String theFragment = fragment == null ? null : substituteMapped(fragment, map); + String theFragment = fragment == null ? null + : substituteMapped(fragment, map, true, fromEncoded); return buildURI(fromEncoded, thePath, theQuery, theFragment); } catch (URISyntaxException ex) { throw new UriBuilderException("URI can not be built", ex); } } + private String substituteVarargs(URITemplate templ, + Object[] values, + int ind, + boolean isQuery, + boolean fromEncoded) { + + Map<String, String> varValueMap = new HashMap<String, String>(); + + // vars in set are properly ordered due to linking in hash set + Set<String> uniqueVars = new LinkedHashSet<String>(templ.getVariables()); + if (values.length < uniqueVars.size()) { + throw new IllegalArgumentException("Unresolved variables; only " + values.length + + " value(s) given for " + uniqueVars.size() + + " unique variable(s)"); + } + int idx = ind; + for (String var : uniqueVars) { + + Object oval = values[idx++]; + if (oval == null) { + throw new IllegalArgumentException("No object for " + var); + } + String value = oval.toString(); + if (fromEncoded) { + value = HttpUtils.encodePartiallyEncoded(value, isQuery); + } else { + value = isQuery ? HttpUtils.queryEncode(value) : HttpUtils.pathEncode(value); + } + + varValueMap.put(var, value); + + } + return templ.substitute(varValueMap); + } - private String substituteMapped(String path, Map<String, ? extends Object> varValueMap) { + private String substituteMapped(String path, Map<String, ? extends Object> varValueMap, + boolean isQuery, boolean fromEncoded) { URITemplate templ = new URITemplate(path); @@ -254,7 +262,20 @@ public class UriBuilderImpl extends UriB + " value(s) given for " + uniqueVars.size() + " unique variable(s)"); } - return templ.substitute(varValueMap); + Map<String, Object> theMap = new LinkedHashMap<String, Object>(); + for (String var : uniqueVars) { + Object oval = varValueMap.get(var); + if (oval == null) { + throw new IllegalArgumentException("No object for " + var); + } + if (fromEncoded) { + oval = HttpUtils.encodePartiallyEncoded(oval.toString(), isQuery); + } else { + oval = isQuery ? HttpUtils.queryEncode(oval.toString()) : HttpUtils.pathEncode(oval.toString()); + } + theMap.put(var, oval); + } + return templ.substitute(theMap); } @Override @@ -500,14 +521,14 @@ public class UriBuilderImpl extends UriB } } - private String buildPath(boolean fromEncoded) { + private String buildPath() { StringBuilder sb = new StringBuilder(); Iterator<PathSegment> iter = paths.iterator(); while (iter.hasNext()) { PathSegment ps = iter.next(); String p = ps.getPath(); if (p.length() != 0 || !iter.hasNext()) { - p = fromEncoded ? new URITemplate(p).encodeLiteralCharacters() : p; + p = new URITemplate(p).encodeLiteralCharacters(false); if (sb.length() == 0 && leadingSlash) { sb.append('/'); } else if (!p.startsWith("/") && sb.length() > 0) { @@ -515,16 +536,16 @@ public class UriBuilderImpl extends UriB } sb.append(p); if (iter.hasNext()) { - buildMatrix(sb, ps.getMatrixParameters(), fromEncoded); + buildMatrix(sb, ps.getMatrixParameters()); } } } - buildMatrix(sb, matrix, fromEncoded); + buildMatrix(sb, matrix); return sb.toString(); } - private String buildQuery(boolean fromEncoded) { - return buildParams(query, '&', fromEncoded); + private String buildQuery() { + return buildParams(query, '&'); } @Override @@ -674,16 +695,18 @@ public class UriBuilderImpl extends UriB * @param fromEncoded if true then values will be decoded * @return stringified params. */ - private String buildParams(MultivaluedMap<String, String> map, char separator, - boolean fromEncoded) { + private String buildParams(MultivaluedMap<String, String> map, char separator) { boolean isQuery = separator == '&'; StringBuilder b = new StringBuilder(); for (Iterator<Map.Entry<String, List<String>>> it = map.entrySet().iterator(); it.hasNext();) { Map.Entry<String, List<String>> entry = it.next(); for (Iterator<String> sit = entry.getValue().iterator(); sit.hasNext();) { String val = sit.next(); - if (fromEncoded || (isQuery && !val.startsWith("{") && !val.endsWith("}"))) { + boolean templateValue = val.startsWith("{") && val.endsWith("}"); + if (!templateValue) { val = HttpUtils.encodePartiallyEncoded(val, isQuery); + } else { + val = new URITemplate(val).encodeLiteralCharacters(isQuery); } b.append(entry.getKey()); if (val.length() != 0) { @@ -703,18 +726,17 @@ public class UriBuilderImpl extends UriB * @param sb buffer to add the matrix part to, will get ';' added if map is not empty * @param map matrix multivalued map */ - private void buildMatrix(StringBuilder sb, MultivaluedMap<String, String> map, - boolean fromEncoded) { + private void buildMatrix(StringBuilder sb, MultivaluedMap<String, String> map) { if (!map.isEmpty()) { sb.append(';'); - sb.append(buildParams(map, ';', fromEncoded)); + sb.append(buildParams(map, ';')); } } private PathSegment replacePathSegment(PathSegment ps) { StringBuilder sb = new StringBuilder(); sb.append(ps.getPath()); - buildMatrix(sb, matrix, false); + buildMatrix(sb, matrix); return new PathSegmentImpl(sb.toString()); } } Modified: cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriInfoImpl.java URL: http://svn.apache.org/viewvc/cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriInfoImpl.java?rev=1452534&r1=1452533&r2=1452534&view=diff ============================================================================== --- cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriInfoImpl.java (original) +++ cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriInfoImpl.java Mon Mar 4 21:23:44 2013 @@ -190,7 +190,7 @@ public class UriInfoImpl implements UriI } UriBuilder ub = UriBuilder.fromPath(sum); objects.addAll(invocation.getTemplateValues()); - uris.add(0, ub.build(objects.toArray()).normalize().getPath()); + uris.add(0, ub.build(objects.toArray()).normalize().getRawPath()); } return uris; } Modified: cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java URL: http://svn.apache.org/viewvc/cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java?rev=1452534&r1=1452533&r2=1452534&view=diff ============================================================================== --- cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java (original) +++ cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java Mon Mar 4 21:23:44 2013 @@ -279,7 +279,7 @@ public final class URITemplate { sb.append(value); } else { throw new IllegalArgumentException("Template variable " + var.getName() - + " has no matching value"); + + " has no matching value"); } } else { sb.append(chunk); @@ -293,13 +293,13 @@ public final class URITemplate { * ex. "a {id} b" will be encoded to "a%20{id}%20b" * @return encoded value */ - public String encodeLiteralCharacters() { + public String encodeLiteralCharacters(boolean isQuery) { final float encodedRatio = 1.5f; StringBuilder sb = new StringBuilder((int)(encodedRatio * template.length())); for (UriChunk chunk : uriChunks) { String val = chunk.getValue(); if (chunk instanceof Literal) { - sb.append(HttpUtils.encodePartiallyEncoded(val, false)); + sb.append(HttpUtils.encodePartiallyEncoded(val, isQuery)); } else { sb.append(val); } Modified: cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriBuilderImplTest.java URL: http://svn.apache.org/viewvc/cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriBuilderImplTest.java?rev=1452534&r1=1452533&r2=1452534&view=diff ============================================================================== --- cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriBuilderImplTest.java (original) +++ cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriBuilderImplTest.java Mon Mar 4 21:23:44 2013 @@ -40,6 +40,76 @@ import org.junit.Test; public class UriBuilderImplTest extends Assert { @Test + public void testBuildWithNonEncodedSubstitutionValue() { + URI uri; + uri = UriBuilder.fromPath("/{a}").build("{}"); + assertEquals("/%7B%7D", uri.toString()); + } + + @Test + public void testBuildWithNonEncodedSubstitutionValue2() { + URI uri; + uri = UriBuilder.fromPath("/{a}").buildFromEncoded("{}"); + assertEquals("/%7B%7D", uri.toString()); + } + + @Test + public void testBuildWithNonEncodedSubstitutionValue3() { + UriBuilder ub = UriBuilder.fromPath("/"); + URI uri = ub.path("{a}").buildFromEncoded("%"); + assertEquals("/%25", uri.toString()); + uri = ub.path("{token}").buildFromEncoded("%", "{}"); + assertEquals("/%25/%7B%7D", uri.toString()); + } + + @Test + public void testBuildWithNonEncodedSubstitutionValue4() { + UriBuilder ub = UriBuilder.fromPath("/"); + URI uri = ub.path("{a}").build("%"); + assertEquals("/%25", uri.toString()); + uri = ub.path("{token}").build("%", "{}"); + assertEquals("/%25/%7B%7D", uri.toString()); + } + + @Test + public void testBuildWithNonEncodedSubstitutionValue5() { + UriBuilder ub = UriBuilder.fromUri("/%25"); + URI uri = ub.build(); + assertEquals("/%25", uri.toString()); + uri = ub.replacePath("/%/{token}").build("{}"); + assertEquals("/%25/%7B%7D", uri.toString()); + } + + @Test + public void testBuildWithNonEncodedSubstitutionValue6() { + UriBuilder ub = UriBuilder.fromPath("/"); + URI uri = ub.path("%").build(); + assertEquals("/%25", uri.toString()); + uri = ub.replacePath("/%/{token}").build("{}"); + assertEquals("/%25/%7B%7D", uri.toString()); + } + + @Test + public void testBuildWithNonEncodedSubstitutionValue7() { + UriBuilder ub = UriBuilder.fromPath("/"); + URI uri = ub.replaceQueryParam("a", "%").buildFromEncoded(); + assertEquals("/?a=%25", uri.toString()); + uri = ub.replaceQueryParam("a2", "{token}").buildFromEncoded("{}"); + assertEquals("/?a=%25&a2=%7B%7D", uri.toString()); + } + + @Test + public void testBuildWithNonEncodedSubstitutionValue8() { + UriBuilder ub = UriBuilder.fromPath("/"); + URI uri = ub.replaceQueryParam("a", "%").build(); + assertEquals("/?a=%25", uri.toString()); + uri = ub.replaceQueryParam("a2", "{token}").build("{}"); + assertEquals("/?a=%25&a2=%7B%7D", uri.toString()); + } + + + + @Test public void testQueryParamWithTemplateValues() { URI uri; uri = UriBuilder.fromPath("/index.jsp").queryParam("a", "{a}").queryParam("b", "{b}") @@ -52,6 +122,11 @@ public class UriBuilderImplTest extends UriBuilder.fromPath("/index.jsp").queryParam("a", "{a}").queryParam("b", "{b}") .build("valueA"); } + + @Test(expected = IllegalArgumentException.class) + public void testQueryParamWithMissingTemplateValues2() { + UriBuilder.fromPath("/index.jsp").queryParam("a", "{a}").build(); + } @Test public void testPathAndQueryParamWithTemplateValues() { @@ -1008,7 +1083,7 @@ public class UriBuilderImplTest extends @Test public void testNullScheme() { - String expected = "//localhost:8080"; + String expected = "localhost:8080"; URI uri = UriBuilder.fromUri("http://localhost:8080";) .scheme(null) .build(); Modified: cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/model/URITemplateTest.java URL: http://svn.apache.org/viewvc/cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/model/URITemplateTest.java?rev=1452534&r1=1452533&r2=1452534&view=diff ============================================================================== --- cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/model/URITemplateTest.java (original) +++ cxf/branches/2.6.x-fixes/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/model/URITemplateTest.java Mon Mar 4 21:23:44 2013 @@ -698,13 +698,13 @@ public class URITemplateTest extends Ass @Test public void testEncodeLiteralCharacters() { URITemplate ut = new URITemplate("a {id} b"); - assertEquals("a%20{id}%20b", ut.encodeLiteralCharacters()); + assertEquals("a%20{id}%20b", ut.encodeLiteralCharacters(false)); } @Test public void testEncodeLiteralCharactersNotVariable() { URITemplate ut = new URITemplate("a {digit:[0-9]} b"); //System.out.println(ut.encodeLiteralCharacters()); - assertEquals("a%20{digit:[0-9]}%20b", ut.encodeLiteralCharacters()); + assertEquals("a%20{digit:[0-9]}%20b", ut.encodeLiteralCharacters(false)); } }
