idno / known

A social publishing platform.
https://withknown.com/opensource
Other
1.06k stars 196 forks source link

twitter avatar issues #753

Closed srcr closed 8 years ago

srcr commented 9 years ago

In my Know installation at home I have an issue with twitter avatar pictures. when I download a picture from my site it will show me the following content:

You are being redirected.

Looks like there is an issue with the download process.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

benwerd commented 9 years ago

Uh oh. Is your site public? Can you point to a page where this is happening? (Are you using brid.gy to get the avatars?)

davidmead commented 9 years ago

I'm seeing the same thing as @srcr.

Looking at the comments left on http://blog.davidjohnmead.com/2015/this-is-why-i-love-indieweb-starring-microformats#comments you can see the gravatars from peoples sites are coming through, but not the ones from Twitter.

screen shot 2015-02-10 at 8 15 52 am

When right-clicking on Aaron's broken image I get this URL http://blog.davidjohnmead.com/file/eb3363dd5d68c3848a72bdc84a97cdc4 , which entered into the browser address bar downloads a file. Launching that file gives me a redirect link.

screen shot 2015-02-10 at 8 20 25 am

Clicking on that, gives me this link https://pbs.twimg.com/profile_images/3657148842/934fb225b84b8fd3effe5ab95bb18005.jpeg which is Aaron's photo.

Not sure what's going on.

srcr commented 9 years ago

@benwerd Indeed the problem that @davidmead describes is the same as mine, only far better described :+1: My sites is in my profile, but In the meantime I downloaded the profile pictures manually so the issues is currently not visible on my site.

mapkyca commented 9 years ago

Looks very much like twitter has changed the URL of the image CDN, but not changed the url extracted by the MF2 parser. Alternatively, it's a bridgy thing, and it's not correctly getting the true url from whatever shim it uses to turn twitter permalinks into MF2 marked up stuff.

In any case, we're attempting to pull the icon and hitting a redirect.

I do think this is primarily a bridgy/twitter interaction issue, but an obvious fix would be to get our code to follow redirects - not sure why it's not already, tbh, but perhaps that's an interaction with our old open_basedir glitch (which iirc @davidmead suffered from).

ghost commented 9 years ago

I have the same problem, actually for Facebook too, but far less often.

paulcmal commented 8 years ago

Could everybody make sure #1096 fixed the problem?

If so, can somebody close the issue?

srcr commented 8 years ago

I can confirm that Twitter images now come in good on my end. Thanks for the update.

mapkyca commented 8 years ago

Can close.

paulcmal commented 8 years ago

1096 was a quick and ugly fix. This was a better fix but @mapkyca's d5bcaae808c69ad499f88c267781fb17ce911252 is even better.

But I've just stumbled upon this and it seems that PHP and libcURL now follow redirections even with open_basedir and safemode enabled, which is a behaviour I had already noticed.

Could anybody confirm this behaviour?

The upstream fix occurred two years ago (PHP 5.6.0). Debian stable (Jessie) now ships with PHP 5.6.14, so the question is: do we need to keep PHP 5.4 compatibility or can we fix this once and for all?

Sorry to reopen the debate!

kylewm commented 8 years ago

fwiw, it looks like ≥ 5.4 covers ~90% of php installations, while ≥ 5.6 is only ~23%.

composer-2015

source

benwerd commented 8 years ago

Yeah, we've got to keep support for 5.4 for a while - although it might be desirable to shim it and use the real solution if we detect that 5.6 is present.

On Wednesday, December 30, 2015, Kyle Mahan notifications@github.com wrote:

fwiw, it looks like ≥ 5.4 covers ~90% of php installations, while ≥ 5.6 is only ~23%.

[image: composer-2015] https://cloud.githubusercontent.com/assets/950127/12054390/17a8181e-aed8-11e5-9acb-536da4b872c2.png

source http://seld.be/notes/php-versions-stats-2015-edition

— Reply to this email directly or view it on GitHub https://github.com/idno/Known/issues/753#issuecomment-168040560.

Ben Werdmuller http://goog_1933028737 benwerd.com | werd.io

+1 (312) 488-9373

paulcmal commented 8 years ago

In the docs, the use of eAccelerator is recommended, and Known's caching only supports Xcache at the moment. But PHP 7 has come out and will soon be rolling out on all distros, and eAccelerator and Xcache only support PHP <= 5.4 and 5.6 respectively.

Sorry if I'm going off-topic here, but shouldn't we try to support new and better technologies? In that particular case, opcode (shipped with PHP >= 5.5) and APCU would do a much better job and the codebase would be compatible with the current releases of PHP, but probably also the next.

I'm aware that a lot of people out there (including sysadmins) use outdated software, but do these people really use ground-breaking concepts and technos such as the IndieWeb and Known? I mean, in all the people hosting Known that we know of, do some people actually use PHP < 5.6?

The Known installation guide states:

Some of the technologies involved are a little bit new, so you may have to ask for your web host to install them specially.

That sounded pretty good to me.

although it might be desirable to shim it and use the real solution if we detect that 5.6 is present.

So what are the options you have in mind? Fix the problem and maintain a seperate branch for backwards-compatibility with PHP 5.4? Add a config option for this?

benwerd commented 8 years ago

I think it's fair to move over to opcode and APCU, for sure.

The problem is, honestly, that most shared hosts - which people overwhelmingly use to self-host - are far behind the latest releases. Some shared hosts even require special tricks to upgrade them from PHP 5.3 to 5.4. It's deeply annoying. (Of course, not all shared hosts are made equal, and Known is sponsored by DreamHost, while many people use Reclaim Hosting in education.)

My suggestion is:

paulcmal commented 8 years ago

My suggestion

Sounds good. And it's possible to do the same here to handle redirections.

Thank you for the explanation. If nobody has anything else to say, I think we can close the issue.

PS: Do you think you could package a holiday release to definitely fix this broken avatar issue for everyone not using the git version?

mapkyca commented 8 years ago

It's XCache rather than APCU purely because that's what I happened to have installed, so that's what I coded first ;)

APCU should be a fairly simple to code.

kylewm commented 8 years ago

This discussion went a fair bit afield of the original issue :) My reading is that the issue was "fixed" but the fix was ugly, and there is a cleaner way to have done it: https://github.com/idno/Known/commit/7eea1d5265782502968e2ef179b45d0cc852861c fixes it by setting cURL params but needs testing by someone with Apache and/or PHP < 5.6 to see if it actually works. Yes yes?

paulcmal commented 8 years ago

Yes yes?

Correct. @mapkyca fixed the bug regarding headers casing (a6cb3af) in the meantime, though.