Don't convert CURLFile params to JSON #89
Conversation
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.
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.
|
👍 |
|
Yes please |
|
👍 yeah! |
|
Bump. |
|
Month bump. |
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.
Small fix to base_facebook.php
Add phpunit as a dependency in composer.json
Check isset before unset
3 huge issues solved.
Fixed curl ssl invalid cert file
Fix bug in CSRF state persistence when using shared sessions.
Make getLoginStatusUrl use /dialog/oauth
|
+1 |
|
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 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. |
|
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? |
|
Done -- 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? — |
|
#110 could have been alleviated with a rebase: |
|
@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. |
|
@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. |
|
It was already closed, otherwise I’d have been happy to :) -- 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. — |
|
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. |
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.