toshipon / oauth-php

Automatically exported from code.google.com/p/oauth-php
MIT License
0 stars 0 forks source link

Fix getallheaders support for FastCGI and other servers then Apache #28

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
You use the function getallheaders() a couple of time in the OAuth library.
This only works in Apache. I've created a function which also works in
Lighttpd and Apache PHP FastCGI.

Here you can find it: http://pastie.org/855575

I've put it in the class OAuthRequest as static method. And replaced all
getallheaders() with OAuthRequest::getAllHeaders();

Original issue reported on code.google.com by michiele...@gmail.com on 5 Mar 2010 at 2:27

GoogleCodeExporter commented 9 years ago
Thanks a lot! I added it to the SVN, with two slight changes:

- added to OAuthRequestLogger instead
- apache_request_headers() is returned as is, without passing through the 
foreach loop.

Unless you have something to add, I'll consider this fixed.

Original comment by brunobg%...@gtempaccount.com on 5 Mar 2010 at 10:12

GoogleCodeExporter commented 9 years ago
Well I wouldn't expect that function in OAuthRequestLogger, because it's not a
logging function, but an utility function used to extract headers from the 
webserver.
So in my opinion I shouldn't add it to the Logger class. I added it the 
OAuthRequest,
because there are the other utility functions as well, as parse_url and stuff 
like
that. So that's where I should expect it.

The apache_request_headers() can be returned as is yes. But please add there a 
ksort
as well. So you will get the same sorting order as with $_SERVER and $_ENV. 
This will
minimize the differences between the 2 methods.

Besides this, I consider it fixed as well.

Original comment by michiele...@gmail.com on 6 Mar 2010 at 9:06

GoogleCodeExporter commented 9 years ago
I agree that Logger is not the most logical place, but it's the lowest common
dependency and it also uses this function. I added the ksort and closing this. 
Thanks
again for the patch.

Original comment by brunobg%...@gtempaccount.com on 8 Mar 2010 at 12:46