New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't convert CURLFile params to JSON #89

Closed
wants to merge 19 commits into
base: master
from

Conversation

Projects
None yet
@philsturgeon
Contributor

philsturgeon commented Aug 29, 2013

PHP 5.5 will throw deprecation warnings if users are uploading files with the previously documented 'source' => '@/foo/bar.jpg', syntax.

The new approach is to use 'source' => new CurlFile('/foo/bar.jpg', 'image/jpeg'),, which obviously avoids accidental uploads (or upload attempts) from user-provided content which starts with a "@" character.

dosercz and others added some commits Nov 30, 2012

Fix bug in CSRF state persistence when using shared sessions.
BaseFacebook loads the stored state in its constructor. However, at
that point, the shared session ID has not yet been initialized, so
getPersistentData() will return data from the non-shared-session cookie. Since
initSharedSession() depends upon state initialized in
BaseFacebook::__construct, just re-initialized the stored
state in the shared session situation.

Added appropriate tests to check CSRF state persistence with and without shared sessions.
Phil Sturgeon
Don't convert CURLFile params to JSON
PHP 5.5 will throw deprecation warnings if users are uploading files with the previously documented `'source' => '@/foo/bar.jpg',` syntax.

The new approach is to use `'source' => new CurlFile('/foo/bar.jpg', 'image/jpeg'),`, which obviously avoids accidental uploads (or upload attempts) from user-provided content which starts with a `"@"` character.
@Rican7

This comment has been minimized.

Show comment
Hide comment
@Rican7

Rican7 commented Aug 29, 2013

👍

@Cangit

This comment has been minimized.

Show comment
Hide comment
@Cangit

Cangit Aug 29, 2013

Yes please

Cangit commented Aug 29, 2013

Yes please

@irazasyed

This comment has been minimized.

Show comment
Hide comment
@irazasyed

irazasyed Aug 29, 2013

Contributor

👍 yeah!

Contributor

irazasyed commented Aug 29, 2013

👍 yeah!

@philsturgeon

This comment has been minimized.

Show comment
Hide comment
@philsturgeon

philsturgeon Sep 9, 2013

Contributor

Bump.

Contributor

philsturgeon commented Sep 9, 2013

Bump.

@philsturgeon

This comment has been minimized.

Show comment
Hide comment
@philsturgeon

philsturgeon Sep 23, 2013

Contributor

Month bump.

Contributor

philsturgeon commented Sep 23, 2013

Month bump.

oyvindkinsey and others added some commits Aug 27, 2013

Make getLoginStatusUrl use /dialog/oauth
To make this a drop in replacement for the old endpoint, we also need
support from the /dialog/oauth endpoint. Specifically, the current
endpoint, when using display=none, always return the response as part of
the fragment, while the PHP SDK needs the signed_request as a query
string argument.
Merge pull request #98 from stevenwoodson/master
Small fix to base_facebook.php
Merge pull request #61 from gary-rafferty/master
Add phpunit as a dependency in composer.json
Merge pull request #46 from dosercz/master
Fixed curl ssl invalid cert file
Merge pull request #59 from JohnnyGoods/master
Fix bug in CSRF state persistence when using shared sessions.
Merge pull request #94 from oyvindkinsey/master
Make getLoginStatusUrl use /dialog/oauth
@czjvic

This comment has been minimized.

Show comment
Hide comment
@czjvic

czjvic commented Oct 17, 2013

+1

@philsturgeon

This comment has been minimized.

Show comment
Hide comment
@philsturgeon

philsturgeon Oct 20, 2013

Contributor

I was speaking with @gfosco and he suggested I should add unit-tests. The trouble is that there are currently no tests covering this sort of functionality at all, and I am simply adding an "or clause" that checks if something is an instance of CurlFile.

There is no possibility this could break any existing functionality, and it definitely works as I've been using this.

Instead of asking me to write a bunch of unit tests for functionality the SDK should already be testing itself, please just merge this in and let it be part of the recode as things move forward.

As I said, Facebook PHP SDK is currently broken in PHP 5.5 and PHP 5.6 is not that far away.

Contributor

philsturgeon commented Oct 20, 2013

I was speaking with @gfosco and he suggested I should add unit-tests. The trouble is that there are currently no tests covering this sort of functionality at all, and I am simply adding an "or clause" that checks if something is an instance of CurlFile.

There is no possibility this could break any existing functionality, and it definitely works as I've been using this.

Instead of asking me to write a bunch of unit tests for functionality the SDK should already be testing itself, please just merge this in and let it be part of the recode as things move forward.

As I said, Facebook PHP SDK is currently broken in PHP 5.5 and PHP 5.6 is not that far away.

@gfosco

This comment has been minimized.

Show comment
Hide comment
@gfosco

gfosco Oct 28, 2013

Contributor

When updated against master, the PR now contains a lot more than your original one line change... Would it be possible for you to submit a new PR with just that one line, and the last comment you made?

Contributor

gfosco commented Oct 28, 2013

When updated against master, the PR now contains a lot more than your original one line change... Would it be possible for you to submit a new PR with just that one line, and the last comment you made?

@philsturgeon

This comment has been minimized.

Show comment
Hide comment
@philsturgeon

philsturgeon Oct 28, 2013

Contributor

Done
#110

-- 
Phil Sturgeon

On 28 October 2013 at 19:23:48, Fosco Marotto (notifications@github.com) wrote:

When updated against master, the PR now contains a lot more than your original one line change... Would it be possible for you to submit a new PR with just that one line, and the last comment you made?


Reply to this email directly or view it on GitHub.

Contributor

philsturgeon commented Oct 28, 2013

Done
#110

-- 
Phil Sturgeon

On 28 October 2013 at 19:23:48, Fosco Marotto (notifications@github.com) wrote:

When updated against master, the PR now contains a lot more than your original one line change... Would it be possible for you to submit a new PR with just that one line, and the last comment you made?


Reply to this email directly or view it on GitHub.

@Rican7

This comment has been minimized.

Show comment
Hide comment

Rican7 commented Oct 29, 2013

@philsturgeon

This comment has been minimized.

Show comment
Hide comment
@philsturgeon

philsturgeon Oct 29, 2013

Contributor

@Rican7 It certainly could have, but creating a fresh branch and using cherry-pick took about 15 seconds. That leaves me unsure why you're posting that.

Contributor

philsturgeon commented Oct 29, 2013

@Rican7 It certainly could have, but creating a fresh branch and using cherry-pick took about 15 seconds. That leaves me unsure why you're posting that.

@Rican7

This comment has been minimized.

Show comment
Hide comment
@Rican7

Rican7 Oct 29, 2013

@philsturgeon Just a tip on how this could have been done in one issue, instead of creating a duplicate, in case someone else comes across this. No disrespect intended.

Rican7 commented Oct 29, 2013

@philsturgeon Just a tip on how this could have been done in one issue, instead of creating a duplicate, in case someone else comes across this. No disrespect intended.

@philsturgeon

This comment has been minimized.

Show comment
Hide comment
@philsturgeon

philsturgeon Oct 29, 2013

Contributor

It was already closed, otherwise I’d have been happy to :)

-- 
Phil Sturgeon

On 29 October 2013 at 11:43:54, Trevor N. Suarez (notifications@github.com) wrote:

@philsturgeon Just a tip on how this could have been done in one issue, instead of creating a duplicate, in case someone else comes across this. No disrespect intended.


Reply to this email directly or view it on GitHub.

Contributor

philsturgeon commented Oct 29, 2013

It was already closed, otherwise I’d have been happy to :)

-- 
Phil Sturgeon

On 29 October 2013 at 11:43:54, Trevor N. Suarez (notifications@github.com) wrote:

@philsturgeon Just a tip on how this could have been done in one issue, instead of creating a duplicate, in case someone else comes across this. No disrespect intended.


Reply to this email directly or view it on GitHub.

@gfosco

This comment has been minimized.

Show comment
Hide comment
@gfosco

gfosco Oct 29, 2013

Contributor

Yeah that was my fault.. Since it wasn't rebased for the last update, I asked for a new PR and closed it. It's not a big deal.

Contributor

gfosco commented Oct 29, 2013

Yeah that was my fault.. Since it wasn't rebased for the last update, I asked for a new PR and closed it. It's not a big deal.

@nabeelsaleem

This comment has been minimized.

Show comment
Hide comment
@nabeelsaleem

nabeelsaleem Apr 17, 2014

@oyvindkinsey
can i do these things with facebook unity sdk. without php server?
Login popu for user.
invite friends already chekmared .
share score.

nabeelsaleem commented on 88ffc93 Apr 17, 2014

@oyvindkinsey
can i do these things with facebook unity sdk. without php server?
Login popu for user.
invite friends already chekmared .
share score.

This comment has been minimized.

Show comment
Hide comment
@e77e44

e77e44 Apr 28, 2014

require_once("facebook.php");

$config = array(
'appId' => 'YOUR_APP_ID',
'secret' => 'YOUR_APP_SECRET',
'fileUpload' => false, // optional
'allowSignedRequest' => false, // optional, but should be set to false for non-canvas apps
);

$facebook = new Facebook($config); معَ تححيات ابن مقيص

e77e44 replied Apr 28, 2014

require_once("facebook.php");

$config = array(
'appId' => 'YOUR_APP_ID',
'secret' => 'YOUR_APP_SECRET',
'fileUpload' => false, // optional
'allowSignedRequest' => false, // optional, but should be set to false for non-canvas apps
);

$facebook = new Facebook($config); معَ تححيات ابن مقيص

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment