search

jdolitsky / AppDotNetPHP

PHP library for the App.net Stream API
34 stars 19 forks source link

Proposal to merge httpPost, httpGet and httpDelete functions in AppDotNet.php #19

Closed cdn closed 11 years ago

cdn commented 11 years ago

These three share much code and code redundancy can be reduced if they are merged.

This really only applies to AppDotNet.php, despite it saying there are two files with changes.

ravisorg commented 11 years ago

Interesting... Something about this is rubbing me the wrong way, but I can't put my finger on it. I feel like we're losing future flexibility or something by merging everything into one function, even though logically that doesn't seem to make much sense. Ok if I think about it for a bit?

cdn commented 11 years ago

It was only a suggestion.

I did it for my own ease of use, given my host's inability to verify the SSL at the moment. I only have to reference the CA certificate once.

ravisorg commented 11 years ago

Ahh now that's a problem we should solve though, the SSL cert thing. Is it that they can't verify, or are they injecting a MITM attack into your outgoing SSL connections? Should we add an option to ignore SSL verification, or would that be a Very Bad Thing(tm)? (I'm thinking very bad, but thoughts anyone?)

jdolitsky commented 11 years ago

sounds like a good idea to me, I imagine that all requests to the API (get post delete) will always follow a similar pattern to each other and this shouldn't be a bad idea.

I was given some advise when I initially started this repo by @voidfiles to use Requests for PHP (http://requests.ryanmccue.info/). The ADN team uses it for their internal PHP client, and it seems to offer a ton of features to control http requests and avoid php-curl. I do like the fact that we're currently not using any dependencies, just a thought

jdolitsky commented 11 years ago

the slogan is "Never touch cURL again."

cdn commented 11 years ago

On the subject of SSL failure:

I added logging to curl in AppDotNet.php and got this :

Fixed with

             curl_setopt($ch, CURLOPT_CAINFO, dirname(__FILE__) . '/DigiCertHighAssuranceEVRootCA.crt');
             curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, true);

addition to AppDotNet.php's httpX() functions

and a visit to https://www.digicert.com/digicert-root-certificates.htm

Re: not using curl directly, I'd like to know why I never found that before, last time I needed a non-curl solution I used an fsockopen-based function pulled from a comment on php.net

ravisorg commented 11 years ago

@jdolitsky holy crap, requests is awesome, I'm definitely bookmarking that one...

ravisorg commented 11 years ago

@cdn ahh I see, your host simply doesn't have a CA set up (or a very old one). I wonder if we could include the app.net API CA in the library if the system CA isn't found...

jdolitsky commented 11 years ago

alright, made an executive decision and committed @cdn 's changes. good stuff