Bug in OAuthRequest.split_headers and .from_request

[Morten Fangel]

Hi,

First of, there is a bug in OAuthRequest.from_request in that the port-
number isn't kept..
Here's a quick table how URI's are parsed and how they should be parsed

URI -> Correct -> Current
http://example.com/ -> http://example.com/ -> http://example.com/
http://example.com:80/ -> http://example.com/ -> http://example.com/
http://example.com:8080/ -> http://example.com:8080/ -> http://example.com/
(WRONG)
https://example.com/ -> https://example.com/ -> https://example.com/
https://example.com:443/ -> https://example.com/ -> https://example.com/
https://example.com:8443/ -> https://example.com:8443/ -> https://example.com/
(WRONG)

So I have attached a patch (20080116_scheme_port_fix.patch) that
attaches :_port_ if
it's any other combo than http:80 or https:443

Secondly OAuthRequest.split_headers is wrong.. It will fail to parse
Auth.-headers like
OAuth realm="",oauth_foo=bar,oauth_baz="bla,rgh"

So I have attached a patch (20081016_split_headers.patch) that changes
the way
split_headers() work, from using strpos/explode to a iterated reg.exp.

And as a third thing, arrays in input-data are.. handled wrong.. (You
get a error trying)
I have yet to fix this.. But I will do it when I get the time..

I am using a PHPUnit test-case to run OAuthRequest through a series of
tests, running
through most of the test-cases that the OAuth.net wiki articles
mentions under signing etc
It's pretty rough, and not really foolproof (works by setting alot of
$_SERVER-variables
tricking the library into parsing it they way I want it to)
In the future I will expand the coverage to all the classes in
OAuth.php, but it takes time..
Lots of time..
Let me know if it's of general interest to get these PHPUnit-files
into SVN..

Also - how about that commit-right.. :P

Regards
-fangel

[Chris Messina]

Ok, I've added you to the repository. Sorry for the delay!

Chris

--
Chris Messina
Citizen-Participant &
Open Web Advocate-at-Large

factoryjoe.com # diso-project.org
citizenagency.com # vidoop.com
This email is: [ ] bloggable [X] ask first [ ] private

[Morten Fangel]

Lovely. Thanks Chris

Does anyone have anything to say about either the patches or
whether the phpunit-tests I'm working on should be commited?

At least someone should try and apply them to see if something
blows up. I don't think they will, but hey.. they might..

-fangel

[Will Norris]

+1 on phpunit tests be in the repository. absolutely!

[Bruno Pedro]

+1 I'm with you. phpunit-tests should be in the repository.

Bruno Pedro

[fangel]

Okay, I've added the PHPUnit tests here
http://oauth.googlecode.com/svn/code/php/tests/

So far only OAuthRequest is covered, and there are currently 3
tests that fail..

1. is because split_headers is broken (so someone test my patch so
we can get it commited)
2. is because the port isn't kept (again, someone test my patch)
3. is because arrays in parameters isn't handled correctly

And I just saw that get_normalized_http_method is updated to keep the
port if necesary, so the only changes needed to from_request is to
send
the port along.. So see the revised patch inlined in this email (can't
get
the web-interface to attach files..)

-fangel

-- 20080118_scheme_port_fix.patch --

--- a/OAuth.php
+++ b/OAuth.php
@@ -186,7 +186,7 @@ class OAuthRequest {/*{{{*/
*/
public static function from_request($http_method=NULL,
$http_url=NULL, $parameters=NULL) {/*{{{*/
$scheme = (!isset($_SERVER['HTTPS']) || $_SERVER['HTTPS'] !=
"on") ? 'http' : 'https';
- @$http_url or $http_url = $scheme . '://' . $_SERVER
['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
+ @$http_url or $http_url = $scheme . '://' . $_SERVER
['HTTP_HOST'] . ':' . $_SERVER['SERVER_PORT'] . $_SERVER
['REQUEST_URI'];
@$http_method or $http_method = $_SERVER['REQUEST_METHOD'];

$request_headers = OAuthRequest::get_headers();

--

On Jan 17, 11:35 pm, Bruno Pedro <bpe...@gmail.com> wrote:
> +1 I'm with you. phpunit-tests should be in the repository.
>
> Bruno Pedro
>