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

Fix image proxy from throwing 404 on initial image load. #4073

Merged
merged 4 commits into from Jul 5, 2017

Conversation

Projects
None yet
4 participants

colinschoen added some commits May 30, 2017

Remove terminating new line
Signed-off-by: Colin Schoen <cschoen@berkeley.edu>
Improved solution
Signed-off-by: Colin Schoen <cschoen@berkeley.edu>
@tinoest

This comment has been minimized.

Show comment
Hide comment
@tinoest

tinoest May 31, 2017

Contributor

I'm not sure this is the best solution reading the whole file, there are lots of places it will return false and those are all checked by is_int, as per the comment in the forum thread this won't check correctly.

I would change it so that a true or false is returned and to check against that.
if ($responseCode != 200) { return false; }

Should allow you to do that along with reverting the last change.

Then a
if (!$response) { // Throw a 404 header('HTTP/1.0 404 Not Found'); exit; }

As you don't seem to care about what the response code was from the curl request.

Contributor

tinoest commented May 31, 2017

I'm not sure this is the best solution reading the whole file, there are lots of places it will return false and those are all checked by is_int, as per the comment in the forum thread this won't check correctly.

I would change it so that a true or false is returned and to check against that.
if ($responseCode != 200) { return false; }

Should allow you to do that along with reverting the last change.

Then a
if (!$response) { // Throw a 404 header('HTTP/1.0 404 Not Found'); exit; }

As you don't seem to care about what the response code was from the curl request.

Return false instead of HTTP error code
Signed-off-by: Colin Schoen <cschoen@berkeley.edu>
@colinschoen

This comment has been minimized.

Show comment
Hide comment
@colinschoen

colinschoen Jun 19, 2017

Member

Agreed, thanks. This should be fixed now.

Member

colinschoen commented Jun 19, 2017

Agreed, thanks. This should be fixed now.

@Oldiesmann Oldiesmann merged commit 2e08ece into SimpleMachines:release-2.1 Jul 5, 2017

2 checks passed

Scrutinizer 1 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Gwenwyfar Gwenwyfar added the Proxy label Mar 4, 2018

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