Skip to content

Commit 80e4baa

Browse files
slandelleslandelle
committed
OAuthSignatureCalculator mustn't re-encodes query params, close AsyncHttpClient#921 
1 parent 9da7919 commit 80e4baa

File tree

2 files changed

+90
-23
lines changed

2 files changed

+90
-23
lines changed

‎src/main/java/com/ning/http/client/oauth/OAuthSignatureCalculator.java

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,34 +128,33 @@ private String encodedParams(long oauthTimestamp, String nonce, List<Param> form
128128
OAuthParameterSet allParameters = new OAuthParameterSet(allParametersSize);
129129

130130
// start with standard OAuth parameters we need
131-
allParameters.add(KEY_OAUTH_CONSUMER_KEY, consumerAuth.getKey());
131+
allParameters.add(KEY_OAUTH_CONSUMER_KEY, UTF8UrlEncoder.encodeQueryElement(consumerAuth.getKey()));
132132
allParameters.add(KEY_OAUTH_NONCE, nonce);
133133
allParameters.add(KEY_OAUTH_SIGNATURE_METHOD, OAUTH_SIGNATURE_METHOD);
134134
allParameters.add(KEY_OAUTH_TIMESTAMP, String.valueOf(oauthTimestamp));
135135
if (userAuth.getKey() != null) {
136-
allParameters.add(KEY_OAUTH_TOKEN, userAuth.getKey());
136+
allParameters.add(KEY_OAUTH_TOKEN, UTF8UrlEncoder.encodeQueryElement(userAuth.getKey()));
137137
}
138138
allParameters.add(KEY_OAUTH_VERSION, OAUTH_VERSION_1_0);
139139

140140
if (formParams != null) {
141141
for (Param param : formParams) {
142-
allParameters.add(param.getName(), param.getValue());
142+
// formParams are not already encoded
143+
allParameters.add(UTF8UrlEncoder.encodeQueryElement(param.getName()), UTF8UrlEncoder.encodeQueryElement(param.getValue()));
143144
}
144145
}
145146
if (queryParams != null) {
146147
for (Param param : queryParams) {
148+
// queryParams are already encoded
147149
allParameters.add(param.getName(), param.getValue());
148150
}
149151
}
150152
return allParameters.sortAndConcat();
151153
}
152154

153-
/**
154-
* Method for calculating OAuth signature using HMAC/SHA-1 method.
155-
*/
156-
public String calculateSignature(String method, Uri uri, long oauthTimestamp, String nonce,
155+
StringBuilder signatureBaseString(String method, Uri uri, long oauthTimestamp, String nonce,
157156
List<Param> formParams, List<Param> queryParams) {
158-
157+
159158
// beware: must generate first as we're using pooled StringBuilder
160159
String baseUrl = baseUrl(uri);
161160
String encodedParams = encodedParams(oauthTimestamp, nonce, formParams, queryParams);
@@ -169,6 +168,16 @@ public String calculateSignature(String method, Uri uri, long oauthTimestamp, St
169168
// and all that needs to be URL encoded (... again!)
170169
sb.append('&');
171170
UTF8UrlEncoder.encodeAndAppendQueryElement(sb, encodedParams);
171+
return sb;
172+
}
173+
174+
/**
175+
* Method for calculating OAuth signature using HMAC/SHA-1 method.
176+
*/
177+
public String calculateSignature(String method, Uri uri, long oauthTimestamp, String nonce,
178+
List<Param> formParams, List<Param> queryParams) {
179+
180+
StringBuilder sb = signatureBaseString(method, uri, oauthTimestamp, nonce, formParams, queryParams);
172181

173182
ByteBuffer rawBase = StringUtils.charSequence2ByteBuffer(sb, UTF_8);
174183
byte[] rawSignature = mac.digest(rawBase);
@@ -230,7 +239,7 @@ public OAuthParameterSet(int size) {
230239
}
231240

232241
public OAuthParameterSet add(String key, String value) {
233-
Parameter p = new Parameter(UTF8UrlEncoder.encodeQueryElement(key), UTF8UrlEncoder.encodeQueryElement(value));
242+
Parameter p = new Parameter(key, value);
234243
allParameters.add(p);
235244
return this;
236245
}

‎src/test/java/com/ning/http/client/oauth/OAuthSignatureCalculatorTest.java

Lines changed: 72 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
*
4040
*/
4141
public class OAuthSignatureCalculatorTest {
42+
4243
private static final String CONSUMER_KEY = "dpf43f3p2l4k3l03";
4344

4445
private static final String CONSUMER_SECRET = "kd94hf93k423kf44";
@@ -52,16 +53,16 @@ public class OAuthSignatureCalculatorTest {
5253
final static long TIMESTAMP = 1191242096;
5354

5455
private static class StaticOAuthSignatureCalculator extends OAuthSignatureCalculator {
55-
56+
5657
private final long timestamp;
5758
private final String nonce;
58-
59+
5960
public StaticOAuthSignatureCalculator(ConsumerKey consumerAuth, RequestToken userAuth, long timestamp, String nonce) {
6061
super(consumerAuth, userAuth);
61-
this.timestamp = timestamp;
62+
this.timestamp = timestamp;
6263
this.nonce = nonce;
6364
}
64-
65+
6566
@Override
6667
protected long generateTimestamp() {
6768
return timestamp;
@@ -72,7 +73,62 @@ protected String generateNonce() {
7273
return nonce;
7374
}
7475
}
75-
76+
77+
// sample from RFC https://tools.ietf.org/html/rfc5849#section-3.4.1
78+
private void testSignatureBaseString(Request request) {
79+
ConsumerKey consumer = new ConsumerKey("9djdj82h48djs9d2", CONSUMER_SECRET);
80+
RequestToken user = new RequestToken("kkk9d7dh3k39sjv7", TOKEN_SECRET);
81+
OAuthSignatureCalculator calc = new OAuthSignatureCalculator(consumer, user);
82+
83+
String signatureBaseString = calc.signatureBaseString(//
84+
request.getMethod(),//
85+
request.getUri(),//
86+
137131201,//
87+
"7d8f3e4a",//
88+
request.getFormParams(),//
89+
request.getQueryParams()).toString();
90+
91+
assertEquals(signatureBaseString, "POST&" //
92+
+ "http%3A%2F%2Fexample.com%2Frequest" //
93+
+ "&a2%3Dr%2520b%26"//
94+
+ "a3%3D2%2520q%26" + "a3%3Da%26"//
95+
+ "b5%3D%253D%25253D%26"//
96+
+ "c%2540%3D%26"//
97+
+ "c2%3D%26"//
98+
+ "oauth_consumer_key%3D9djdj82h48djs9d2%26"//
99+
+ "oauth_nonce%3D7d8f3e4a%26"//
100+
+ "oauth_signature_method%3DHMAC-SHA1%26"//
101+
+ "oauth_timestamp%3D137131201%26"//
102+
+ "oauth_token%3Dkkk9d7dh3k39sjv7%26"//
103+
+ "oauth_version%3D1.0");
104+
}
105+
106+
@Test(groups = "fast")
107+
public void testSignatureBaseStringWithProperlyEncodedUri() {
108+
109+
Request request = new RequestBuilder("POST")//
110+
.setUrl("http://example.com/request?b5=%3D%253D&a3=a&c%40=&a2=r%20b")//
111+
.addFormParam("c2", "")//
112+
.addFormParam("a3", "2 q")//
113+
.build();
114+
115+
testSignatureBaseString(request);
116+
}
117+
118+
@Test(groups = "fast")
119+
public void testSignatureBaseStringWithRawUri() {
120+
121+
// note: @ is legal so don't decode it into %40 because it won't be encoded back
122+
// note: we don't know how to fix a = that should have been encoded as %3D but who would be stupid enough to do that?
123+
Request request = new RequestBuilder("POST")//
124+
.setUrl("http://example.com/request?b5=%3D%253D&a3=a&c%40=&a2=r b")//
125+
.addFormParam("c2", "")//
126+
.addFormParam("a3", "2 q")//
127+
.build();
128+
129+
testSignatureBaseString(request);
130+
}
131+
76132
// based on the reference test case from
77133
// http://oauth.pbwiki.com/TestCases
78134
@Test(groups = "fast")
@@ -99,10 +155,11 @@ public void testPostCalculateSignature() {
99155
formParams.add(new Param("file", "vacation.jpg"));
100156
formParams.add(new Param("size", "original"));
101157
String url = "http://photos.example.net/photos";
102-
final Request req = new RequestBuilder("POST")
103-
.setUri(Uri.create(url))
104-
.setFormParams(formParams)
105-
.setSignatureCalculator(calc).build();
158+
final Request req = new RequestBuilder("POST")//
159+
.setUri(Uri.create(url))//
160+
.setFormParams(formParams)//
161+
.setSignatureCalculator(calc)//
162+
.build();
106163

107164
// From the signature tester, POST should look like:
108165
// normalized parameters: file=vacation.jpg&oauth_consumer_key=dpf43f3p2l4k3l03&oauth_nonce=kllo9940pd9333jh&oauth_signature_method=HMAC-SHA1&oauth_timestamp=1191242096&oauth_token=nnch734d00sl2jdk&oauth_version=1.0&size=original
@@ -135,14 +192,15 @@ public void testGetWithRequestBuilder() {
135192
queryParams.add(new Param("size", "original"));
136193
String url = "http://photos.example.net/photos";
137194

138-
final Request req = new RequestBuilder("GET")
139-
.setUri(Uri.create(url))
140-
.setQueryParams(queryParams)
141-
.setSignatureCalculator(calc).build();
195+
final Request req = new RequestBuilder("GET")//
196+
.setUri(Uri.create(url))//
197+
.setQueryParams(queryParams)//
198+
.setSignatureCalculator(calc)//
199+
.build();
142200

143201
final List<Param> params = req.getQueryParams();
144202
assertEquals(params.size(), 2);
145-
203+
146204
// From the signature tester, the URL should look like:
147205
//normalized parameters: file=vacation.jpg&oauth_consumer_key=dpf43f3p2l4k3l03&oauth_nonce=kllo9940pd9333jh&oauth_signature_method=HMAC-SHA1&oauth_timestamp=1191242096&oauth_token=nnch734d00sl2jdk&oauth_version=1.0&size=original
148206
//signature base string: GET&http%3A%2F%2Fphotos.example.net%2Fphotos&file%3Dvacation.jpg%26oauth_consumer_key%3Ddpf43f3p2l4k3l03%26oauth_nonce%3Dkllo9940pd9333jh%26oauth_signature_method%3DHMAC-SHA1%26oauth_timestamp%3D1191242096%26oauth_token%3Dnnch734d00sl2jdk%26oauth_version%3D1.0%26size%3Doriginal

0 commit comments

Comments
 (0)