Comments (17)
It looks like the problem might stem from the escapable [] characters in the
parameter name, rather than the
use of POST. I tried adding the following test in SignatureBaseStringTest:
@Test
public void shouldWorkWithBracketsInParameterName() throws Exception {
HttpRequest request = mock(HttpRequest.class);
when(request.getMethod()).thenReturn("GET");
when(request.getRequestUrl()).thenReturn("http://examplebrackets.com");
HttpParameters params = new HttpParameters();
params.put("a[]", "1");
SignatureBaseString sbs = new SignatureBaseString(request, params);
assertEquals("GET&http%3A%2F%2Fexamplebrackets.com%2F&a%255B%255D%3D1", sbs.generate());
}
(I got the hopefully correct SBS from
http://googlecodesamples.com/oauth_playground/)
This test failed as follows:
org.junit.ComparisonFailure: expected:<...om%2F&a%255B%255D%3D[1]> but
was:<...om%2F&a%255B%255D%3D[]>
So the value of the parameter was missing, just like in the originally reported
failure. I tracked down the
problem to the following sequence of operations in HttpParameters:
public String getAsQueryString(Object key) {
StringBuilder sb = new StringBuilder();
key = OAuth.percentEncode((String) key); // the provided key is first encoded
Set<String> values = wrappedMap.get(key); // and then used to grab the value
if (values == null) { // so we get value as null
return key + "=";
}
Note that this is a problem because in
SignatureBaseString.normalizeRequestParameters you have:
Iterator<String> iter = requestParameters.keySet().iterator();
for (int i = 0; iter.hasNext(); i++) {
String param = iter.next();
if (OAuth.OAUTH_SIGNATURE.equals(param) || "realm".equals(param)) {
continue;
}
if (i > 0) {
sb.append("&");
}
sb.append(requestParameters.getAsQueryString(param)); // <-- the key provided here is straight from
the Map!
}
So, normalizeRequestParameters will send to getAsQueryString exactly the value
for the key present in the
TreeMap, and yet getAsQueryString will encode it again before retrieving the
corresponding value for the
parameter - which means that unless encoding is a no-op for the particular
parameter key, we're doing
something wrong.
To try to resolve the bug, I tried switching the order of the operations in
getAsQueryString like this:
Set<String> values = wrappedMap.get(key);
key = OAuth.percentEncode((String) key);
While that fixed my added test, it broke one of yours (RequestParametersTest):
params.put("a b", "c d", true);
assertEquals("a%20b=c%20d", params.getAsQueryString("a b"));
Taking a cue off that test, I returned the library code as it was and tried
changing my test:
@Test
public void shouldWorkWithBracketsInParameterName() throws Exception {
HttpRequest request = mock(HttpRequest.class);
when(request.getMethod()).thenReturn("GET");
when(request.getRequestUrl()).thenReturn("http://examplebrackets.com");
HttpParameters params = new HttpParameters();
params.put("a[]", "1", true);
SignatureBaseString sbs = new SignatureBaseString(request, params);
assertEquals("GET&http%3A%2F%2Fexamplebrackets.com%2F&a%255B%255D%3D1", sbs.generate());
}
...but then I get the following failure:
org.junit.ComparisonFailure: expected:<...brackets.com%2F&a%25[5B%255D%3D1]>
but
was:<...brackets.com%2F&a%25[255B%25255D%3D]>
In this case there was additional escaping (which may or may not be correct,
I'm not sure) but the parameter
value was still missing.
I'm a little stuck now because:
1. I don't know what you had in mind for the percentDecode parameter. When
should a parameter be percent
encoded before being inserted in the map, and when should it not?
2. I am actually using the Apache HttpClient in my application, and I don't yet
understand how that all
interacts with the core code. Perhaps all of this has no bearing whatsoever on
code that uses HttpClient, but
the similarity in symptoms is rather suspicious.
Thoughts?
Original comment by [email protected]
on 3 Jun 2010 at 12:06
from oauth-signpost.
I'm also having this problem with signpost-1.2.1.1 on Android using Apache
Commons -- square brackets "[]" fail with both GET and POST requests, requests
without them work fine.
Original comment by [email protected]
on 11 Aug 2010 at 6:05
from oauth-signpost.
This issue is KILLING me. Is there anything that can be done? Perhaps you can
post a patch for what you describe as
"I tried switching the order of the operations in getAsQueryString like this"
so we can test it out in our apps. It may be breaking a test, but that test
line you include looks nonsensical to me (though it lacks context, so....).
Anyway, I'd really like to see this resolved somehow. Otherwise, I must rewrite
my code using another library :(
Original comment by [email protected]
on 13 Aug 2010 at 2:23
from oauth-signpost.
OK I gave it a try. Attached is a new .jar, see if it works for you.
My changes were committed to my fork of signpost, in a new branch 1.2.1.2. See
the changes here:
http://github.com/srajko/signpost/commit/1cd201f8b8e1168c48b8cd3fcc6391f706fa469
f
Please let me know if this helps out any.
Original comment by [email protected]
on 13 Aug 2010 at 5:18
Attachments:
from oauth-signpost.
Still fails for me.
Original comment by [email protected]
on 13 Aug 2010 at 7:16
from oauth-signpost.
So, I am not wise in the ways of OAuth, but I played around with it and what
worked for me was NOT percent encoding the keys:
public String getAsQueryString(Object key) {
StringBuilder sb = new StringBuilder();
//key = OAuth.percentEncode((String) key);
//Set<String> values = wrappedMap.get(key);
Set<String> values = wrappedMap.get(key);
//key = OAuth.percentEncode((String) key);
if (values == null) {
return key + "=";
}
Iterator<String> iter = values.iterator();
while (iter.hasNext()) {
sb.append(key + "=" + iter.next());
if (iter.hasNext()) {
sb.append("&");
}
}
return sb.toString();
}
I don't know maven at all, but I compiled with "mvn package -DskipTests"
This makes only a tiny bit of sense to me.....
Original comment by [email protected]
on 13 Aug 2010 at 2:50
from oauth-signpost.
not percent encoding ever in getAsQueryString breaks other test cases, but I'm
not sure how relevant that is for end-user use cases. The change I made makes
percent encoding optional (true by default), and I changed one place where
getAsQueryString is called to opt for not percent encoding. That makes the
test case I added pass, but there might be other places where calls to
getAsQueryString might need to get fixed.
Did the jar I posted work for you, bjorn?
Original comment by [email protected]
on 13 Aug 2010 at 2:58
from oauth-signpost.
stjepan: I made my last post without realizing you had posted in between. oops.
I will try your jar. I wonder if the right fix is simply to avoid escaping
brackets?
Just a thought: is anybody having trouble with a non-ruby server?
bjorn
Original comment by [email protected]
on 13 Aug 2010 at 4:18
from oauth-signpost.
stjepan: no, your fix did not work :(
so, I know, "works for me" does not mean that you've found the right way to do
it, but if we pretend for a moment that it does, I think the right way to do it
is to alter signpost-core in some way that resembles my suggestion above.
Original comment by [email protected]
on 13 Aug 2010 at 4:30
from oauth-signpost.
[deleted comment]
from oauth-signpost.
One way or another, there is a serious bug in the HttpParameters.put function,
which should read:
public String put(String key, String value, boolean percentEncode) {
key = percentEncode ? OAuth.percentEncode(key) : key;
SortedSet<String> values = wrappedMap.get(key);
if (values == null) {
values = new TreeSet<String>();
wrappedMap.put( key, values);
}
if (value != null) {
value = percentEncode ? OAuth.percentEncode(value) : value;
values.add(value);
}
return value;
}
rather than:
public String put(String key, String value, boolean percentEncode) {
SortedSet<String> values = wrappedMap.get(key);
if (values == null) {
values = new TreeSet<String>();
wrappedMap.put( percentEncode ? OAuth.percentEncode(key) : key, values);
}
if (value != null) {
value = percentEncode ? OAuth.percentEncode(value) : value;
values.add(value);
}
return value;
}
which will cause problems when there are multiple parameters with the same name
that need to be escaped, like
playlist[track][][id]=12
playlist[track][][id]=15
playlist[track][][id]=34
playlist[track][][id]=127
in soundcloud.
Original comment by [email protected]
on 13 Aug 2010 at 8:34
from oauth-signpost.
Hi Bjorn, Casper
I just got a chance to test the modified jar - and it works for me.
Potentially good news is, I POSTED THE WRONG JAR! :-) (sorry about that). The
commonshttp jar is actually no different than the 1.2.1.1. version. It is the
core jar that changed.
Please see if this one works for you.
(Bjorn, I'm not sure about the change you posted in your last comment. The
original code seems correct to me. I think the keys and values should always
be stored in a percent encoded state - the question is whether they are already
supplied as percent encoded (or percent encoding is a no-op for the supplied
strings), in which case they should not be percent encoded *again*.)
Original comment by [email protected]
on 14 Aug 2010 at 4:13
Attachments:
from oauth-signpost.
Stjepan,
I discovered this bug experimentally, and fixing cause my problems to go away.
However, you can also follow the logic of the original code where percentEncode
is true. By eliminating the percentEncode variable, it becomes:
public String put(String key, String value ) {
SortedSet<String> values = wrappedMap.get(key);
if (values == null) {
values = new TreeSet<String>();
wrappedMap.put( OAuth.percentEncode(key), values);
}
if (value != null) {
value = OAuth.percentEncode(value);
values.add(value);
}
return value;
}
In the case where key.equals( OAuth.percentEncode(key) ) is false, then:
SortedSet<String> values = wrappedMap.get(key);
will give values = null, because no key could have ever gotten into the map
that wasn't percent encoded. Why? Because the only put call in the code is
wrappedMap.put( OAuth.percentEncode(key), values);
With values == null, this call:
wrappedMap.put( OAuth.percentEncode(key), values);
will replace any preexisting OAuth.percentEncode(key) in the wrappedMap without
retaining it's values. If we repeat the call with the same key, regardless of
value, the prior value is lost.
This is definitely not the intent of the OAuth spec, and causes failures any
time there are multiple, identical keys that require escaping.
Original comment by [email protected]
on 14 Aug 2010 at 4:45
from oauth-signpost.
Yes, you are right. Let me add that change into the patched JAR so it can also
handle multiple parameters with the same key. I'll also add an appropriate test
to make sure this always works correctly.
Original comment by [email protected]
on 14 Aug 2010 at 4:51
from oauth-signpost.
OK, I've added a test for multiple identical keys that require escaping, and
your change that makes the test pass (BTW, sorry about being initially
skeptical about your fix - there was a crucial part that I missed when reading
it).
I am now attaching a new JAR which now includes both my fix for keys that need
escaping, and your fix for multiple keys that need escaping. All old signpost
tests still pass, so hopefully we didn't break anything.
Original comment by [email protected]
on 14 Aug 2010 at 5:25
- Changed state: Started
Attachments:
from oauth-signpost.
Working for me now -- many thanks.
Original comment by [email protected]
on 14 Aug 2010 at 7:21
from oauth-signpost.
signpost-core-1.2.1.2.jar works great... thanx a lot
Original comment by [email protected]
on 28 Nov 2011 at 3:11
from oauth-signpost.
Related Issues (20)
- Request: RSA-SHA1 Signing HOT 4
- retrieveRequestToken(consumer, null) fails with signpost 1.2.1.1 HOT 1
- retrieveRequestToken returns an error of 500 HOT 4
- POST support for DefaultOAuthConsumer HOT 1
- OAuthGoogleExample not executing HOT 2
- After Java upgrade: "Server returned HTTP response code: 411 for URL HOT 10
- For Android Twitter Usage HOT 3
- SignatureBaseString havent sort requestParameters
- Oauth java.lang.ClassNotFoundException: org.apache.http.HttpRequest
- Enhancement : adding license text in tarball
- didn't support HTTP GET method in this framework
- two-legged OAuth HOT 3
- Signing fails when multiple parameters with the same key are used HOT 1
- consumer key with special characters causes signpost to generate incorrect signature HOT 4
- Doesnt work on 3.2, tested on motorola xoom 3.2 HOT 5
- NULL exception after CommonsHttpOAuthProvider is de-serialized
- oauth.signpost.exception.OAuthCommunicationException: Communication with the service provider failed: null HOT 5
- incorrect get signing when params include space
- URL query parameter without values causes failure HOT 1
- oauth-signpost don't include the license file
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from oauth-signpost.