From ca0076c043c20ad50e0acd638ad6fa70600b0b73 Mon Sep 17 00:00:00 2001 From: krogoth Date: Wed, 2 Feb 2011 23:46:40 +0100 Subject: [PATCH] Refactored scribe 1.1.0 to handdle properly encodings as per RFC5849. This is the first try for comment. --- .../org/scribe/builder/api/FacebookApi.java | 2 +- .../extractors/BaseStringExtractorImpl.java | 28 +++++++++++-- .../scribe/extractors/TokenExtractorImpl.java | 4 +- src/main/java/org/scribe/model/Request.java | 9 +++- src/main/java/org/scribe/model/Verifier.java | 7 ++-- src/main/java/org/scribe/utils/URLUtils.java | 41 +++++++++++++++++-- .../java/org/scribe/model/RequestTest.java | 6 ++- .../java/org/scribe/model/VerifierTest.java | 7 ---- .../java/org/scribe/utils/URLUtilsTest.java | 14 +++---- 9 files changed, 86 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/scribe/builder/api/FacebookApi.java b/src/main/java/org/scribe/builder/api/FacebookApi.java index c945520..5adf445 100644 --- a/src/main/java/org/scribe/builder/api/FacebookApi.java +++ b/src/main/java/org/scribe/builder/api/FacebookApi.java @@ -11,6 +11,6 @@ public class FacebookApi extends DefaultApi20 public String getAuthorizationUrl(OAuthConfig config) { Preconditions.checkValidUrl(config.getCallback(), "Must provide a valid url as callback. Facebook does not support OOB"); - return String.format(AUTHORIZE_URL, config.getApiKey(), URLUtils.percentEncode(config.getCallback())); + return String.format(AUTHORIZE_URL, config.getApiKey(), URLUtils.urlEncodeWrapper(config.getCallback())); } } diff --git a/src/main/java/org/scribe/extractors/BaseStringExtractorImpl.java b/src/main/java/org/scribe/extractors/BaseStringExtractorImpl.java index f18c62c..f2612b8 100644 --- a/src/main/java/org/scribe/extractors/BaseStringExtractorImpl.java +++ b/src/main/java/org/scribe/extractors/BaseStringExtractorImpl.java @@ -32,11 +32,31 @@ public class BaseStringExtractorImpl implements BaseStringExtractor private String getSortedAndEncodedParams(OAuthRequest request) { Map params = new HashMap(); - params.putAll(request.getQueryStringParams()); - params.putAll(request.getBodyParams()); - params.putAll(request.getOauthParameters()); + Map tmp = null; + //params.putAll(request.getQueryStringParams()); + tmp = request.getQueryStringParams(); + for (String key:tmp.keySet()){ + params.put( + URLUtils.percentEncode(key), + URLUtils.percentEncode(tmp.get(key)) + ); + } + tmp = request.getBodyParams(); + for (String key:tmp.keySet()){ + params.put( + URLUtils.percentEncode(key), + URLUtils.percentEncode(tmp.get(key)) + ); + } + tmp = request.getOauthParameters(); + for (String key:tmp.keySet()){ + params.put( + URLUtils.percentEncode(key), + URLUtils.percentEncode(tmp.get(key)) + ); + } params = MapUtils.sort(params); - return URLUtils.percentEncode(URLUtils.formURLEncodeMap(params)); + return URLUtils.percentEncode(URLUtils.concatSortedPercentEncodedParams(params)); } private void checkPreconditions(OAuthRequest request) diff --git a/src/main/java/org/scribe/extractors/TokenExtractorImpl.java b/src/main/java/org/scribe/extractors/TokenExtractorImpl.java index 2bc0aa0..ec131a2 100644 --- a/src/main/java/org/scribe/extractors/TokenExtractorImpl.java +++ b/src/main/java/org/scribe/extractors/TokenExtractorImpl.java @@ -27,8 +27,8 @@ public class TokenExtractorImpl implements RequestTokenExtractor, AccessTokenExt Matcher matcher = Pattern.compile(TOKEN_REGEX).matcher(response); if (matcher.matches()) { - String token = URLUtils.percentDecode(matcher.group(1)); - String secret = URLUtils.percentDecode(matcher.group(2)); + String token = URLUtils.urlDecodeWrapper(matcher.group(1)); + String secret = URLUtils.urlDecodeWrapper(matcher.group(2)); return new Token(token, secret); } else { diff --git a/src/main/java/org/scribe/model/Request.java b/src/main/java/org/scribe/model/Request.java index 1d6b677..fcec4b3 100644 --- a/src/main/java/org/scribe/model/Request.java +++ b/src/main/java/org/scribe/model/Request.java @@ -144,6 +144,7 @@ class Request * Get a {@link Map} of the query string parameters. * * @return a map containing the query string parameters + * @throws UnsupportedEncodingException */ public Map getQueryStringParams() { @@ -156,7 +157,11 @@ class Request for (String param : query.split("&")) { String pair[] = param.split("="); - params.put(pair[0], pair[1]); + String valuePart = ""; + if (pair.length > 1) { + valuePart = URLUtils.urlDecodeWrapper(pair[1]); + } + params.put(URLUtils.urlDecodeWrapper(pair[0]),valuePart ); } } params.putAll(querystringParams); @@ -165,7 +170,7 @@ class Request catch (MalformedURLException mue) { throw new OAuthException("Malformed URL", mue); - } + } } /** diff --git a/src/main/java/org/scribe/model/Verifier.java b/src/main/java/org/scribe/model/Verifier.java index f1dee62..8dff90c 100644 --- a/src/main/java/org/scribe/model/Verifier.java +++ b/src/main/java/org/scribe/model/Verifier.java @@ -1,7 +1,5 @@ package org.scribe.model; -import org.scribe.utils.*; - /** * Represents an OAuth verifier code. * @@ -19,7 +17,10 @@ public class Verifier */ public Verifier(String value) { - this.value = URLUtils.percentDecode(value); + if (value == null) { + throw new IllegalArgumentException(); + } + this.value = value; } public String getValue() diff --git a/src/main/java/org/scribe/utils/URLUtils.java b/src/main/java/org/scribe/utils/URLUtils.java index 30abc59..893cd48 100644 --- a/src/main/java/org/scribe/utils/URLUtils.java +++ b/src/main/java/org/scribe/utils/URLUtils.java @@ -31,7 +31,7 @@ public class URLUtils } /** - * Turns a map into a form-url-encoded string (key=value&key2=value2) + * Turns a map into a form-url-encoded string (key=value&key2&key3=value) * * @param map any map * @return form-url-encoded string @@ -51,7 +51,10 @@ public class URLUtils { encodedString.append(PARAM_SEPARATOR); } - encodedString.append(percentEncode(key)).append(PAIR_SEPARATOR).append(percentEncode(map.get(key))); + encodedString.append(urlEncodeWrapper(key)); + if (map.get(key).length() > 0) { + encodedString.append(PAIR_SEPARATOR).append(urlEncodeWrapper(map.get(key))); + } } return encodedString.toString(); } @@ -81,12 +84,31 @@ public class URLUtils } /** - * Percent decodes a string + * URL encodes a string + * + * @param plain + * @return percent encoded string + */ + public static String urlEncodeWrapper(String string) + { + Preconditions.checkNotNull(string, "Cannot encode null string"); + try + { + return URLEncoder.encode(string, UTF_8); + } + catch (UnsupportedEncodingException uee) + { + throw new IllegalStateException(ERROR_MSG, uee); + } + } + + /** + * URL decodes a string * * @param string percent encoded string * @return plain string */ - public static String percentDecode(String string) + public static String urlDecodeWrapper(String string) { Preconditions.checkNotNull(string, "Cannot decode null string"); try @@ -133,4 +155,15 @@ public class URLUtils return string.replace(ch, toCh); } } + + public static String concatSortedPercentEncodedParams(Map params) { + StringBuilder target = new StringBuilder(); + for (String key:params.keySet()){ + target.append(key); + target.append(PAIR_SEPARATOR); + target.append(params.get(key)); + target.append(PARAM_SEPARATOR); + } + return target.deleteCharAt(target.length() - 1).toString(); +} } diff --git a/src/test/java/org/scribe/model/RequestTest.java b/src/test/java/org/scribe/model/RequestTest.java index 750f004..f7681dd 100644 --- a/src/test/java/org/scribe/model/RequestTest.java +++ b/src/test/java/org/scribe/model/RequestTest.java @@ -17,7 +17,7 @@ public class RequestTest connection = new ConnectionStub(); postRequest = new Request(Verb.POST, "http://example.com"); postRequest.setConnection(connection); - getRequest = new Request(Verb.GET, "http://example.com?qsparam=value&other=value+with+spaces"); + getRequest = new Request(Verb.GET, "http://example.com?qsparam=value&other+param=value+with+spaces"); getRequest.setConnection(connection); } @@ -34,6 +34,8 @@ public class RequestTest assertEquals(2, getRequest.getQueryStringParams().size()); assertEquals(0, postRequest.getQueryStringParams().size()); assertTrue(getRequest.getQueryStringParams().containsKey("qsparam")); + assertTrue(getRequest.getQueryStringParams().get("qsparam").equals("value")); + assertTrue(getRequest.getQueryStringParams().get("other param").equals("value with spaces")); } @Test @@ -52,7 +54,7 @@ public class RequestTest postRequest.addBodyParameter("param", "value"); postRequest.addBodyParameter("param two", "value with spaces"); postRequest.send(); - assertEquals("param%20two=value%20with%20spaces¶m=value", postRequest.getBodyContents()); + assertEquals("param+two=value+with+spaces¶m=value", postRequest.getBodyContents()); assertTrue(connection.getHeaders().containsKey("Content-Length")); } diff --git a/src/test/java/org/scribe/model/VerifierTest.java b/src/test/java/org/scribe/model/VerifierTest.java index 509ace8..47ab4ab 100644 --- a/src/test/java/org/scribe/model/VerifierTest.java +++ b/src/test/java/org/scribe/model/VerifierTest.java @@ -1,17 +1,10 @@ package org.scribe.model; -import static org.junit.Assert.*; import org.junit.*; public class VerifierTest { - @Test - public void shouldParse() - { - Verifier verifier = new Verifier("p8k%2BGIjIL9PblXq%2BpH6LmT9l"); - assertEquals(verifier.getValue(), "p8k+GIjIL9PblXq+pH6LmT9l"); - } @Test(expected = IllegalArgumentException.class) public void shouldThrowErrorForNullString() diff --git a/src/test/java/org/scribe/utils/URLUtilsTest.java b/src/test/java/org/scribe/utils/URLUtilsTest.java index 8a8be2a..5b264c4 100644 --- a/src/test/java/org/scribe/utils/URLUtilsTest.java +++ b/src/test/java/org/scribe/utils/URLUtilsTest.java @@ -16,7 +16,7 @@ public class URLUtilsTest params.put("key with spaces", "value with spaces"); params.put("&symbols!", "#!"); - String expected = "key=value&key%20with%20spaces=value%20with%20spaces&%26symbols%21=%23%21"; + String expected = "key=value&key+with+spaces=value+with+spaces&%26symbols%21=%23%21"; assertEquals(expected, URLUtils.formURLEncodeMap(params)); } @@ -41,7 +41,7 @@ public class URLUtilsTest { String toDecode = "this+is+a+test+%26%5E"; String expected = "this is a test &^"; - assertEquals(expected, URLUtils.percentDecode(toDecode)); + assertEquals(expected, URLUtils.urlDecodeWrapper(toDecode)); } @Test @@ -50,7 +50,7 @@ public class URLUtilsTest String plain = "!*'();:@&=+$,/?#[]"; String encoded = "%21%2A%27%28%29%3B%3A%40%26%3D%2B%24%2C%2F%3F%23%5B%5D"; assertEquals(encoded, URLUtils.percentEncode(plain)); - assertEquals(plain, URLUtils.percentDecode(encoded)); + assertEquals(plain, URLUtils.urlDecodeWrapper(encoded)); } @Test @@ -79,7 +79,7 @@ public class URLUtilsTest public void shouldThrowExceptionIfStringToDecodeIsNull() { String toDecode = null; - URLUtils.percentDecode(toDecode); + URLUtils.urlDecodeWrapper(toDecode); } @Test(expected = IllegalArgumentException.class) @@ -103,7 +103,7 @@ public class URLUtilsTest public void shouldAppendParametersToSimpleUrl() { String url = "http://www.example.com"; - String expectedUrl = "http://www.example.com?param1=value1¶m2=value%20with%20spaces"; + String expectedUrl = "http://www.example.com?param1=value1¶m2=value+with+spaces"; Map params = new HashMap(); params.put("param1", "value1"); @@ -117,7 +117,7 @@ public class URLUtilsTest public void shouldAppendParametersToUrlWithQuerystring() { String url = "http://www.example.com?already=present"; - String expectedUrl = "http://www.example.com?already=present¶m1=value1¶m2=value%20with%20spaces"; + String expectedUrl = "http://www.example.com?already=present¶m1=value1¶m2=value+with+spaces"; Map params = new HashMap(); params.put("param1", "value1"); @@ -142,6 +142,6 @@ public class URLUtilsTest String encoded = "oauth_verifier=7aEP%2BjNAwvjc0mjhqg0nuXPf"; String expected = "oauth_verifier=7aEP+jNAwvjc0mjhqg0nuXPf"; - Assert.assertEquals(expected, URLUtils.percentDecode(encoded)); + Assert.assertEquals(expected, URLUtils.urlDecodeWrapper(encoded)); } }