Major code bloat, no standalone release, doesn't support PHP 5.3, base SDK still insecure

[cubiclesoft]

I understand the need for this SDK for those who need functionality beyond signing in. However, "Login with Facebook" is what the vast majority of the applications out there are doing with the PHP SDK. Therefore, this SDK is overly bloated for the most common use-case. From such a perspective, the older SDK:

https://github.com/facebookarchive/facebook-php-sdk

Is far better for simple sign in. That SDK is a couple of PHP classes that work under PHP 5.2 and later. No need for Composer, namespaces, or anything fancy. Load the files and get the job done.

PHP 5.3 is the minimum version that this SDK should be supporting as evidenced by the right-hand column on the php.net website. Until 5.3 disappears from the homepage and the associated binaries from the download pages on php.net (archive/museum don't count), all library software developers should continue to support PHP 5.3. Using the versions available on php.net as a guide is a rational, reasonable approach to PHP userland software development.

Some of us prefer "standalone" builds to autoloaders. HTML Purifier, for example, is a large library that offers a standalone build. Consider adding a "login only" standalone build of the SDK.

In addition, this SDK reinvents the wheel. The previous SDK required cURL. You probably added the other HTTP client option for "when cURL isn't available" because you didn't realize that there are nearly complete cURL emulation layers out there:

https://github.com/cubiclesoft/ultimate-web-scraper

You should revert all changes to the HTTP client interface and go back to just using cURL so you don't have to maintain yet another HTTP library. Yes, there are hosts out there without cURL, but don't reinvent the wheel. Just find a few good cURL emulation layers and point the handful of users with problems at those solutions. Code you write is code you have to maintain - and most raw HTTP implementations are broken in a wide variety of subtle ways that will keep coming back to haunt you (been there, done that).

Also, stop inventing random number generators. Your CSRF defense logic is poorly written but very few developers are going to delve that deep into the Facebook PHP SDK to discover that they need to override your logic with something actually secure (let alone have the appropriate technical/security background to do so). The previous SDK suffered the same problem of relying on easily guessed PRNG logic. Sigh.

If you need to include a generator with your code, at least use code like:

https://github.com/cubiclesoft/php-csprng

That is, throw an exception if you can't locate a suitable entropy source.

At any rate, there should be a lightweight version of this SDK that you officially maintain for those who just want to do "Login with Facebook" and nothing else. Clearly you don't want to maintain the other version any more (7 open pull requests, a closed issue tracker, a comment indicating "dead project" status, a different GitHub account, and no updates since). However, there needs to be something better for developers to utilize than this that leaves those still supporting PHP 5.3 (for rational, sensible reasons) without a viable SDK despite forcing Facebook API v2.0 (and later) down our throats sometime next year and only this SDK supports those versions of the API. Some of us who maintain libraries and other software products like to get a head start on upgrading before things break, but the PHP 5.4 minimum requirement in this case means that we can't.

[yguedidi]

• About HTTP client interface, if you don't like provided implementations, use your own one, that why interfaces are made for. An interface is a extension point, if you want you can implement FacebookHttpable (v4.0) to use your Ultimate Web Scraper.
• About entropy, you're actually right, but things should be done with an interface, like HTTP clients.
• About PHP 5.3, is it really "rational, sensible reasons" to use a no more maintained PHP version since two months, even for security issues? And you know, PHP 5.4 released 2 years, 8 months ago..

[SammyK]

Hey @cubiclesoft - thanks for the feedback!

"Login with Facebook" is what the vast majority of the applications out there are doing with the PHP SDK.

I don't have any analytics on what people are using the SDK for so I can't make a claim one way or another about what feature is used most in the SDK. I wonder if the guys at Facebook know. But my hunch is that you're probably right about most developers just using it for a quick "login with Facebook" link and grabbing the user's name & email.

the older SDK is far better for simple sign in.

The old SDK was full of bugs and needed heavy refactoring to fix them. For example, if you were to get an access token from a redirect, it would get overwritten by the outdated cookie from the Javascript SDK on the next page load since the SDK would fall back to that by default due to really crazy spaghetti code.

No need for Composer,

You don't need Composer to use the SDK.

namespaces, or anything fancy.

And namespaces aren't fancy (but that's just my opinion). :)

Load the files and get the job done.

You can do the same without Composer in v4.0 of the SDK:

require __DIR__ . '/path/to/facebook-php-sdk-v4/autoload.php';

The only difference is that you have to do all the annoying use statements at the top:

use Facebook\FacebookSession;
use Facebook\FacebookRequest;
use Facebook\GraphUser;
use Facebook\FacebookRequestException;

However in v4.1 of the SDK (to be released soon hopefully!), you'll be able to do pretty much everything from the main Facebook\Facebook super service:

require __DIR__ . '/path/to/facebook-php-sdk-v4/autoload.php';

$fb = new Facebook\Facebook([
  'app_id' => '{app-id}',
  'app_secret' => '{app-secret}',
  ]);

And it takes care of handling all the internal classes for you.

PHP 5.3 is the minimum version that this SDK should be supporting as evidenced by the right-hand column on the php.net website.

PHP 5.3 reached official EOL (end of life) on August 14, 2014 so it is no longer supported and no new releases are bing made. So the minimum version that this SDK should actually be supporting is one that is being actively maintained for bug fixes and security updates.

I understand that there are a lot of shared hosts out there still on PHP 5.3 so that really sucks if you have to develop on those. But those servers are eventually starting to upgrade. And as PHP gets on a more consistent release cycle, hopefully we'll see servers upgrade more frequently in the future.

But upgrades are always a pain. It's the nature of the beast. But we can't can't support a stale, potentially insecure version of PHP in order to support old servers.

Some of us prefer "standalone" builds to autoloaders.

I'm curious: what's the main different between the old SDK:

require __DIR__ . '/path/to/facebook-php-sdk/src/facebook.php';

$fb = new Facebook(array(
  'appId'  => '{app-id}',
  'secret' => '{app-secret}',
  ));

...and v4.1...

require __DIR__ . '/path/to/facebook-php-sdk-v4/autoload.php';

$fb = new Facebook\Facebook([
  'app_id' => '{app-id}',
  'app_secret' => '{app-secret}',
  ]);

Other than the Facebook\ namespace prefix that you have an issue with? Just like v3, v4 supports Composer but it is not required.

The Facebook\Facebook super service in v4.1 is pretty new so maybe you haven't seen it yet. Here's the documentation for it. I think you might like it. :)

most raw HTTP implementations are broken in a wide variety of subtle ways that will keep coming back to haunt you

Yep! I'm right there with you on that point. In fact there have been lots of PR's and issues related to supporting cURL. I suggested using something like Guzzle, but I'm thinking that you would be opposed to that for some reason. :)

Ultimately if we switched to Guzzle (which uses cURL with a stream wrapper fallback) then we would require the use of composer. But we could offer a "built" version of the SDK for those who don't use composer. @gfosco - thoughts? :)

You should revert all changes to the HTTP client interface and go back to just using cURL so you don't have to maintain yet another HTTP library.

How would this be beneficial? Most of the issues related to the HTTP client have been with cURL specifically.

Your CSRF defense logic is poorly written

This has been brought up before and was improved on. But if you're saying it could be better, let's see a pull request with the something better! :)

I'm going to be submitting a RandomStringGeneratorInterface PR to v4.1 soon (among other interfaces) so that if a developer had their own tried-and-tested CSPRSG solution they wanted to use, they could easily implement it.

throw an exception if you can't locate a suitable entropy source.

That's an excellent idea.

there should be a lightweight version of this SDK that you officially maintain

What features would the lightweight version need to support and realistically, what code would it need to include?

When you say it needs to support '"Login with Facebook" and nothing else', the are a number of contexts that one can be logged into Facebook: 1) redirect URL, 2) signed request sent from POST within canvas context, 3) signed request obtained from cookie set by JavaScript SDK. Just supporting those contexts alone is quite a bit of functionality.

Also - can you point to specific things that you feel are bloat?

despite forcing Facebook API v2.0 (and later) down our throats sometime next year

Welcome to the Facebook Development Platform. :)

Interested to hear back from you! :)

[cubiclesoft]

I'm curious: what's the main different between the old SDK:

require DIR . '/path/to/facebook-php-sdk/src/facebook.php';

$fb = new Facebook(array( 'appId' => '{app-id}', 'secret' => '{app-secret}', ));

...and v4.1...

require DIR . '/path/to/facebook-php-sdk-v4/autoload.php';

$fb = new Facebook\Facebook([ 'app_id' => '{app-id}', 'app_secret' => '{app-secret}', ]);

Other than the Facebook\ namespace prefix that you have an issue with? Just like v3, v4 supports >Composer but it is not required.

I think you misunderstood what I meant by 'standalone' because I briefly fixated on namespaces. Standalone builds are basically the core product packaged up into as few files as possible (usually one PHP file). That way PHP doesn't waste a bunch of time loading up and processing a bunch of files that are generally needed as well as repeatedly almost throwing an exception but happens to get saved at the last second by the autoloader. You went from a single file containing everything to an explosion of files on the system of which a number of them are loaded anyway for most circumstances. Other projects I use offer standalone builds. That's all I was getting at.

PHP 5.3 reached official EOL (end of life) on August 14, 2014 so it is no longer supported and no new releases are bing made. So the minimum version that this SDK should actually be supporting is one that is being actively maintained for bug fixes and security updates.

If you think PHP 5.3 shouldn't be supported, you should approach the PHP group about removing 5.3 from the front page and the available downloads and archiving it in their "museum". They are more likely to listen to a large entity like Facebook making the request this soon after EOL'ing than an individual. I'd be happy to back down from my position about supporting PHP 5.3 at that point. Until it fully moves into museum, including the Windows binaries (they forgot to do that for PHP 5.2), PHP 5.3 should be universally supported by all library developers regardless of what anyone thinks about it.

How would this be beneficial? Most of the issues related to the HTTP client have been with cURL specifically.

I didn't know that. Ignore my suggestion. My library overlays your library automatically anyway on hosts that are missing cURL and your library prioritizes cURL if available.

v4.1 sounds like an improvement over what I'm seeing now. However, all software is vaporware until it is officially released.

Watched the Nomad talk (only ~130 video views - hmm...). It could have been done without the distracting MS Paint show and, perhaps as a result, also didn't explain some of the more important deprecations that I just discovered yesterday. Facebook should consider a "soft launch" period for 1.0 apps to migrate to 2.0 beyond just announcing it on your developer website that no developer will ever visit again after they got their "Login with Facebook" 1.0 app working. As a result, the current strategy will cause the vast majority of developers who use/rely on 1.0 to not discover 2.0 until April 30, 2015. A possible solution would be to temporarily break all 1.0 apps for a day (or two) during the work week in an obvious way sometime before April 30th to get each individual developer's attention. Developers are extremely lazy creatures that won't upgrade anything until something actually breaks. I only discovered 2.0 by accident. Had that not happened, I would have been humming along completely oblivious to the changes happening at Facebook until April 30th when Facebook login would suddenly break across both my and my users' infrastructure.

[SammyK]

Standalone builds are basically the core product packaged up into as few files as possible (usually one PHP file).

Maybe you could make a phar.

If you think PHP 5.3 shouldn't be supported, you should approach the PHP group about removing 5.3 from the front page.

PHP internals have already announced that they are not supporting it anymore.

They are more likely to listen to a large entity like Facebook

I don't work for Facebook. I'm just a random guy who is volunteering his time to the open source community. :)

Watched the Nomad talk (only ~130 video views - hmm...)

What's your point? It was posted less than a week ago and it was my first talk ever so I won't be drawing people en mass. :)

It could have been done without the distracting MS Paint show

I used Photoshop. :)

also didn't explain some of the more important deprecations that I just discovered yesterday

It seems like you missed the point of the talk - the Facebook development platform changes all the time. You gotta just embrace that fact.

Developers are extremely lazy creatures that won't upgrade anything until something actually breaks.

Don't worry, it will.

[SammyK]

@gfosco - What are your thoughts on having a built version of the SDK? Maybe a shell script that builds a phar? And I think you could automatically compile it every time you tag it using a git hook but I've never done that before. :)

[yguedidi]

I'm :-1: on that, PHAR is more for bin package like composer and many others, not for an SDK

[cubiclesoft]

They are more likely to listen to a large entity like Facebook

I don't work for Facebook. I'm just a random guy who is volunteering his time to the open source community. :)

That's rather misleading. You have commit level access to this repo under github.com/facebook and this repo is linked to from the Facebook developer website and therefore gives the repo serious exposure and credibility. From my perspective, until you said the above, you looked like a Facebook employee (regardless of whether or not you are paid for your work) and therefore assumed as much.

-1 on PHAR packaging. +1 on standalone build.

[SammyK]

You have commit level access to this repo under github.com/facebook

Hum... seems you're missing how this whole process works. This is an open source project where anyone can fork & pull. Try it out for yourself, it's fun! :)

-1 on PHAR packaging.

:+1: I'm with you and @yguedidi on this one too.

+1 on standalone build.

Are you suggesting taking all the code and cramming into one giant class, one file and no autoloading?

[yguedidi]

:-1: for a standalone build, got forward and use Composer :)

[SammyK]

got forward and use Composer :)

:+1: :+1: :+1:

[gfosco]

I don't think the audience for a standalone build is large enough to warrant the development and maintenance. There's a couple interesting things in this thread (PRNG, Login only use-case) which I'd love to discuss over pull requests... Thanks. Closing this.