Some meaningless text...
|Resolved||• mitch1977||T2000 Image proxying in Remarkup should not activate unless the response is HTTP 200 with valid image data|
|Wontfix||skyronic||T4108 Allow setting the instance title|
Implementation wise, this means using an HTTPFuture to try to fetch an image to get this error code? I guess with caching this will be okay but it feels a bit aggressive to me. (Also, I could imagine pages timing out sometimes if they have a lot of images.)
Sorry, this description is bad.
If files.enable-proxy is enabled in config, we activate a remarkup rule that triggers on URIs ending in .jpg, .png, .gif, etc -- PhabricatorRemarkupRuleProxyImage that appear in remarkup areas.
This calls PhabricatorFileProxyImage::getProxyImageURI(...) on the URI, which generates a link to /file/proxy/?uri=.... That endpoint (PhabricatorFileProxyController) checks for a cache of the image, and downloads and caches it otherwise, then issues a redirect to it.
- Move the proxy lookup into PhabricatorRemarkupRuleProxyImage, check the response code, and get rid of /file/proxy/ (original vision of this task), so bogus URI-looking strings that happen to end in .png hit some sort of "this isn't valid" cache; or
- delete everything I just mentioned and we'll call it a day.
The goal being to make this:
...render literally, or as a link, rather than the broken image it currently renders as:
But TBQH it's so easy to upload files now that "delete everything" seems entirely reasonable.
I went with the delete everything approach. This feature was pretty fancy to begin with so I figure its better to remove it on balance and push people towards uploading more images and not linking externally.