search

ianmacl / patchtester

13 stars 53 forks source link

notices #1

Open infograf768 opened 13 years ago

infograf768 commented 13 years ago

I get these:

Notice: Undefined index: request_header in /Library/WebServer/Documents/trunk1.7/libraries/joomla/client/github.php on line 173

Warning: usort() [function.usort]: The argument should be an array in /Library/WebServer/Documents/trunk1.7/administrator/components/com_patchtester/models/pulls.php on line 102

Warning: Invalid argument supplied for foreach() in /Library/WebServer/Documents/trunk1.7/administrator/components/com_patchtester/models/pulls.php on line 104

Warning: Invalid argument supplied for foreach() in /Library/WebServer/Documents/trunk1.7/administrator/components/com_patchtester/views/pulls/tmpl/default.php on line 51

elkuku commented 13 years ago

heh - that's the same issue I got... Maybe you could change in libraries/joomla/client/github.php function sendRequest() CURLOPT_SSL_VERIFYPEER from true to false ?

I tried different certs with no avail..

ianmacl commented 13 years ago

Yeah that's the verification error. I suppose we can change VERIFYPEER to false here until I fix it upstream in the library.

ianmacl commented 13 years ago

So I pushed a change that should fix this. I tested on XAMPP WinXP and it worked. If somebody else can confirm that the fix works I will close this as resolved.

jlleblanc commented 13 years ago

Changing CURLOPT_SSL_VERIFYPEER from true to false fixes the issue here on my Mac with a MacPorts complied MAMP stack.

elkuku commented 13 years ago

Well, I can not confirm that the fix works on windows but I can confirm that it works on Linux - perfect :)

I'm totally impressed - there is so much potential in this thingy..

ianmacl commented 13 years ago

Joe can you confirm that the fix I pushed fixes the issue on your Mac?

infograf768 commented 13 years ago

@test No more Notices.

The pull request https://github.com/joomla/joomla-cms/pull/3 is correctly displayed and clicking on it displays the github pull for it.

Clicking on Apply Patch gives:

500 - An error has occurred.

Not connected to server.

jlleblanc commented 13 years ago

@ianmacl just tested with a fresh copy and yes, the notices are gone. However, I'm getting the same 500 error as @infograf768 when applying a patch.

elkuku commented 13 years ago

I can reproduce this issue on a windows box - it occurs when the extension php_openssl.dll is not loaded in your php.ini

On a Mac ? No clue. Is PHP compiled with OpenSSL support ?

Could you check your php_info() and make sure that OpenSSL support is enabled ?

Also, Ian, the 500 comes from an unhandled exception thrown in JHttp.. should we implement a try..catch in the controller and also throw meaningful exception instead of just returning false ?

infograf768 commented 13 years ago

On Mac, with MAMP or Native Apache with php, SSL Support is enabled.

jlleblanc commented 13 years ago

OpenSSL is enabled on my Mac setup as well. On Oct 14, 2011, at 11:58 AM, Nikolai Plath wrote:

I can reproduce this issue on a windows box - it occurs when the extension php_openssl.dll is not loaded in your php.ini

Ona Mac ? No clue.

Could you check your php_info() and make sure that OpenSSL support is enabled ?

Also, Ian, the 500 comes from an unhandled exception thrown in JHttp.. should we implement a try..catch in the controller and also throw meaningful exception instead of just returning false ?

Reply to this email directly or view it on GitHub: https://github.com/ianmacl/patchtester/issues/1#issuecomment-2408852

elkuku commented 13 years ago

JHttp is really unhelpful, eating up valuable error messages the Joomla! way...

try
{
    $connection = $this->connect($uri);
}
catch (Exception $e)
{
    return false;
}

The connect() method doesn't even throw an exception, nor it checks the $err and $errno variables - platform bug :(

Would it make sense to use cURL, now that the https problem seems to be resolved (i hope..) ?

edit ;)

Could you try the following code right in administrator/components/com_patchtester/patchtester.php or in any other place where it gets executed ?

$res = fsockopen('ssl://github.com', 443, $errno, $err, 5);
var_dump($err);
var_dump($errno);
var_dump($res);
ianmacl commented 13 years ago

I wonder if it is the allow_url_fopen setting. Is that enabled?

jlleblanc commented 13 years ago

Yes, allow_url_fopen is enabled on my machine.

On Oct 14, 2011, at 2:13 PM, ianmacl wrote:

I wonder if it is the allow_url_fopen setting. Is that enabled?

Reply to this email directly or view it on GitHub: https://github.com/ianmacl/patchtester/issues/1#issuecomment-2410306

infograf768 commented 13 years ago

allow_url_fopen is also enabled here.

elkuku commented 13 years ago

Could you test again with the latest changes, so this issue might be closed (hopefully) ?

jlleblanc commented 13 years ago

Tested, no longer get the notices in the current version.