Image module reports missing images as error 500 (server error) instead of 404 (not found).

Testbot wants git diff patches ;)

Please update to 7.8.

Blast! My patch was against HEAD.

#6: source_image_exists-1226512-6.patch queued for re-testing.

Makes sense to me.

+++ b/modules/image/image.module
@@ -709,6 +709,12 @@ function image_style_deliver($style, $scheme) {
+  if (!is_file($image_uri)) {

To my knowledge, file_exists() is way faster than is_file() and has way less unintended side-effects.

Overall, however, this patch could use some more eyes before getting RTBC again.

I'm with @manarth file_exists return true with directories and symbolic links

Only as a curiosity, IS_FILE is faster than FILE_EXISTS, if the file exists xD

IS_FILE versus FILE_EXISTS test - 10000 iterations.

FILE_EXISTS with a good file: 0.10488700866699 ms.
FILE_EXISTS with a bad file: 0.36173105239868 ms.
IS_FILE with a good file: 0.0040671825408936 ms.
IS_FILE with a bad file: 0.36448192596436 ms.

Attached the performance test files.

ok, checking whether the file is actually a file makes sense.

Two more things though:

1) I'm not sure I understand why this check happens that late in the request in a delivery callback? (which is pretty much one of the very last things being invoked during a request) I would have expected that a fundamental check like this happens at the very beginning of the request (in the page callback) instead. Perhaps this check even exists already, but doesn't work correctly?

2) It looks like this expectation can be easily asserted in a test.

3) Generating and rendering a full-blown 404 HTML page seems like a lot of overhead, given that in 99% of all cases, the output will not be visible (and the browser renders a red X for a missing image instead).

#1: 1226512-image-check_source_image_exists.patch queued for re-testing.

It clearly makes sense to issue a 404 instead of a 500 error when the file does not exists.
I also think a call to drupal_not_found() should be added when checking that the style defined and the scheme is valid. As it is right now you get a 200 OK status when you request /sites/default/files/styles/my_custom_style/zzzzz.png (an inexistant file).

The patch looks good and works as expected. Changing the drupal_not_found() to a lighter alternative could be useful as well (the same way status 500 is handled):

    //drupal_not_found();
    drupal_add_http_header('Status', '404 Not found');
    print t('Image not found.');

Thanks! This looks like a sensible fix; however, as far as I can see:

1) We still need tests.
2) We need a fix in D8 before we backport to D7.

Marking needs work.

Looks like this is a duplicate of #927138: Fail image generation with 404 instead of 500, when source file is missing