Issue 66 in oauth-php: OAuthRequester unexpectedly attaches php://input parameters
oaut...@googlecode.com
Owner: ----
Labels: Type-Defect Priority-Medium
New issue 66 by kris0476: OAuthRequester unexpectedly attaches php://input
parameters
http://code.google.com/p/oauth-php/issues/detail?id=66
What steps will reproduce the problem?
1. User submits a form to a PHP script on server A with form variables x, y
and z. It's a POST request with variables x, y, z in the request body.
2. While processing this request, server A uses oauth-php to make a
two-legged call to server B with parameters p, q, r:
$params = array('p'=>1, 'q'=>1, 'r'=>1);
$request = new OAuthRequester($url, $method, $params);
$result = $request->doRequest();
What is the expected output? What do you see instead?
Server B should receive variables p, q and r, but in fact receives p, q, r,
x, y and z. OAuthRequest::getRequestBody attaches the parameters from the
original request body before signing and transmitting the whole thing.
What version of the product are you using? On what operating system?
r155, CentOS 5, PHP 5.2
Please provide any additional information below.
Since I'm only using the library as an OAuth consumer, I was able to work
around the problem by putting return ''; on the first line of
OAuthRequest::getRequestBody. This workaround probably breaks the server
functionality but that's not actually a problem for me.
At a glance, OAuthRequest::getRequestBody should only be called when we are
verifying a request, and not when we are in the consumer role. Since both
OAuthRequester and OAuthVerifier extend OAuthRequest, a real solution might
involve letting OAuthRequest know if it should try to read php://input or
if it should strictly rely on the $body argument of its constructor.
oaut...@googlecode.com
Status: Accepted
Comment #1 on issue 66 by brun...@corollarium.com: OAuthRequester
unexpectedly attaches php://input parameters
http://code.google.com/p/oauth-php/issues/detail?id=66
Ok, this is a bug (and a wonderful bug report, thanks). But I think it's
being caused by something else. Could you please post the $method variable?
I think you may be using lowercase 'post' instead of 'POST' and this may be
triggering the bug.
oaut...@googlecode.com
Comment #2 on issue 66 by kris0476: OAuthRequester unexpectedly attaches
http://code.google.com/p/oauth-php/issues/detail?id=66
Thank you for looking into it.
I'm using an uppercase 'POST' for $method. You can see my consumer code
here:
http://github.com/sugestio/sugestio-php/blob/master/SugestioClient.php#L78
addConsumption defines $method and $params before calling the execute
method at line 193.
oaut...@googlecode.com
Comment #3 on issue 66 by brun...@corollarium.com: OAuthRequester
http://code.google.com/p/oauth-php/issues/detail?id=66
I've studied the code and I don't understand why this is happening to you.
OAuthRequester inherits from OAuthRequestSigner, and the constructor for
OAuthRequestSigner overrides the $body variable. I just sent a patch for
lowercase 'post' (bug corrected, thanks, unfortunately not yours) in r160.
The only possible place for this bug seems to be OAuthRequest.php:122:
$parameters .= $this->getRequestBody();
Would you be kind and make a test for me? Please comment this line (and
remove that return ''; from getRequestBody) and see if this fixes the
problem. This part of the code is not mine and I don't understand why it is
there. I contacted the original developer, let's see if he can enlighten
me...
oaut...@googlecode.com
Comment #4 on issue 66 by kris0476: OAuthRequester unexpectedly attaches
http://code.google.com/p/oauth-php/issues/detail?id=66
Yes, commenting out line 122 solves the problem in r155, thanks.
I thought that code was there so that OAuthRequestVerifier could get the
POST parameters directly from the input stream rather than through
$_REQUEST or $_POST. I'm not in a position right now to test if the server
still works with line 122 removed...
oaut...@googlecode.com
Comment #5 on issue 66 by brun...@corollarium.com: OAuthRequester
http://code.google.com/p/oauth-php/issues/detail?id=66
Ok, I got a reply from the original developer and this is being handled
incorrectly. He suggested a possible patch and I'll work on it ASAP.
oaut...@googlecode.com
Comment #6 on issue 66 by brun...@corollarium.com: OAuthRequester
http://code.google.com/p/oauth-php/issues/detail?id=66
I did not forget about it, just haven't had time lately. It's not a trivial
fix, so it will take some time. Sorry. Here's what Marc, the original
developer, said to me, if anybody is interested in tackling this:
The code indeed doesn't check correctly if it should construct from the
parameters or from the arguments.
Maybe the best is to rewrite the function:
if ($method === null)
{
... fetch all arguments from the current request
}
else
{
... fetch from the parameters, set sensible defaults
}
Right now all assignments are interwoven, and incorrectly so.
For example there is an assignment to $headers, which overwrites the
$headers from the parameters.
I think the above split in the logic should help solve the problem.
oaut...@googlecode.com
Comment #7 on issue 66 by archul...@seqcentral.com: OAuthRequester
http://code.google.com/p/oauth-php/issues/detail?id=66
I just ran into this issue myself when using the consumer as a proxy for
AJAX calls. I concur that it is necessary to differentiate whether
OAuthRequest is a parent of OAuthRequestSigner (as a consumer) or
OAuthRequestVerifier (as a server). My workaround has been to add another
variable to OAuthRequest::__construct() rather than use "$method". And then
skip the POST/PUT checks appropriately.
OAuthRequest
function __construct ($signer, $uri = null, $method = null, $parameters
= '', $headers = array(), $body = null)
{
...
// Skip the POST/PUT if signing/making a request
if (! $signer) {
// If this is a post then also check the posted variables
if (strcasecmp($method, 'POST') == 0)
{
...
else if (strcasecmp($method, 'PUT') == 0)
{
...
}
}
From OauthRequestVerifier:
parent::__construct(FALSE, $uri, $method ...
From OauthRequestSigner:
parent::__construct(TRUE, $uri, $method ...
oaut...@googlecode.com
Comment #8 on issue 66 by glademil...@gmail.com: OAuthRequester
This may have consequences I don't know about but it has worked so far for
me. If you make the constructor in OAuth Request Read as
function __construct ( $uri = null, $method = null, $parameters = '',
{
$this->headers = $headers;
// Store the values, prepare for oauth
$this->uri = $uri;
$this->body = $body;
$this->parseUri($parameters);
$this->parseHeaders();
$this->transcodeParams();
}
and the constructor in OAuthRequestVerifier read as
function __construct ( $uri = null, $method = null, $params = null )
{
if ($params) {
$encodedParams = array();
foreach ($params as $key => $value) {
if (preg_match("/^oauth_/", $key)) {
continue;
}
$encodedParams[rawurlencode($key)] = rawurlencode($value);
}
$this->param = array_merge($this->param, $encodedParams);
}
$this->store = OAuthStore::instance();
if (is_object($_SERVER))
{
// Tainted arrays - the normal stuff in anyMeta
if (!$method) {
$method = $_SERVER->REQUEST_METHOD->getRawUnsafe();
}
if (empty($uri)) {
$uri = $_SERVER->REQUEST_URI->getRawUnsafe();
$proto = (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS']
== 'on') ? 'https' : 'http';
if (strpos($uri, "://") === false) {
$uri = sprintf('%s://%s%s', $proto,
$_SERVER->HTTP_HOST->getRawUnsafe(), $uri);
}
}
}
else
{
// non anyMeta systems
if (!$method) {
if (isset($_SERVER['REQUEST_METHOD'])) {
$method = $_SERVER['REQUEST_METHOD'];
}
else {
$method = 'GET';
}
}
$proto = (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS']
== 'on') ? 'https' : 'http';
if (empty($uri)) {
if (strpos($_SERVER['REQUEST_URI'], "://") !== false) {
$uri = $_SERVER['REQUEST_URI'];
}
else {
$uri = sprintf('%s://%s%s', $proto, $_SERVER['HTTP_HOST'],
$_SERVER['REQUEST_URI']);
}
}
}
$headers = OAuthRequestLogger::getAllHeaders();
$this->method = strtoupper($method);
$parameters = '';
if (strcasecmp($method, 'POST') == 0)
{
if ($this->getRequestContentType() == 'multipart/form-data')
{
// Get the posted body (when available)
if (!isset($headers['X-OAuth-Test']))
{
$parameters .= $this->getRequestBodyOfMultipart();
}
}
if ($this->getRequestContentType()
== 'application/x-www-form-urlencoded')
{
// Get the posted body (when available)
if (!isset($headers['X-OAuth-Test']))
{
$parameters .= $this->getRequestBody();
}
}
else
{
$body = $this->getRequestBody();
}
else if (strcasecmp($method, 'PUT') == 0)
{
}
parent::__construct($uri, $method,$parameters);
OAuthRequestLogger::start($this);
}
Then in all the cases I've tried the problem is fixed.
oaut...@googlecode.com
Comment #9 on issue 66 by Nagy.Att...@gmail.com: OAuthRequester
For those, who are looking for a workaround, I found the following:
If you set the $_SERVER['CONTENT_TYPE'] to null, or something that isn't
multipart/form-data or application/x-www-form-urlencoded then the problem
does not occur anymore.
Not sure, if the project is still maintained, but here's my proposed fix:
I've overwritten the getRequestBody() and getRequestBodyOfMultipart() in
the OAuthRequester class, and they both return null. This way the original
logic can be kept in the base class, but the OAuthRequester will stop
reading the post input and so it won't add it to the query parameters.
Attachments:
oauth-php_request_body_as_parameters_fix.patch 1.8 KB
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings