Switching file storage to anything other than public/private breaks image styles

Not sure if this is actually critical, since it is one subsystem, and a more uncommon (but still important) case. Moving to major priority.

The comment doesn't wrap at 80 chars.
I'm leaving at "needs review" for reviews on the actual patch/implementation.

I don't have a way to test this with an S3 account or other similar stream wrapper, however this looks like a good idea to me and I like the approach taken here.

We don't want to assume that people are only every going to use private:// and public:// for displaying images on their site and since public:// is really the anomaly here basing the logic around checking for public makes the most sense and gives us the most flexibility.

No, I don't think this is the right approach.

We should have some other flag on a stream wrapper to indicate if it's public or private - we can have multiple public or private schemes that are local or remote - the 2 core provides must not be hard-coded this way.

Possibly we should define additional constants in stream_wrappers.inc?

I'm running into a similar problem. I also agree with #12.

An additional thing I've uncovered is that if you send a path to a direct file, it chokes as well; for instance, if you try image_style_url('large', 'modules/image/sample.png'), giving an error: Fatal error: Call to a member function getDirectoryPath() on a non-object in /var/www/d7/modules/image/image.module on line 808. If nothing else, we should change the documentation to reflect the reality, which is that it's not a $path it's looking for, but rather a $uri to a public:// or private:// stream.

Adding a private/public constant to the 'type' constants could overload its definition. A separate parameter, 'private' => TRUE, could be a better route.

Having this definition is essential to set up a flexible private file system that allows alternate schemes to be selected but ensures that they are implementing a private scheme.

See #1025124: Remove cruft from theme_image_style() and image_style_url()

This has been fixed in 7 and 8.

The patch in #9 was not applied to D7 or D8, but the main issue was apparently fixed in another issue. A link to that other issue would probably be helpful. :)

I have reproduced this problem with trivial stream wrapper that is public: copy/paste but handles different directory (also inside drupal root). It seems to me that the problem is in image_menu() hook. Namely,

$directory_path = file_stream_wrapper_get_instance_by_scheme('public')->getDirectoryPath();
$items[$directory_path . '/styles/%image_style'] = array( ....

Thus, if the file storage is neither public, nor private Drupal won't call image_style_deliver() callback. After I have implemented similar callback in my module the styles start work as they are supposed to. The callback in my module is just a wrapper around image_style_deliver, without any extra processing:

    $directory_path = file_stream_wrapper_get_instance_by_scheme('myscheme')->getDirectoryPath();
    $items[$directory_path . '/styles/%image_style'] = array(
        'title' => 'Generate image style',
        'page callback' => 'mymodule_image_style_deliver',
        'page arguments' => array(count(explode('/', $directory_path)) + 1),

.............

function mymodule_image_style_deliver($style, $scheme) {
    if(function_exists('image_style_deliver')) {
        $args = func_get_args();
        array_shift($args);
        array_shift($args);
        $target = implode('/', $args);
        image_style_deliver($style, $scheme, $target);
    }
}

Probably, it is necessary to use image field approach (see image.field.inc image_field_settings_form() - I use Drupal 7.12) and create routes for all appropriate file storages. For example,

  foreach (file_get_stream_wrappers(STREAM_WRAPPERS_WRITE_VISIBLE) as $scheme => $stream_wrapper) {
    $directory_path = file_stream_wrapper_get_instance_by_scheme($scheme)->getDirectoryPath();
    $items[$directory_path . '/styles/%image_style'] = array( ....
  }

Optionally, add this info to the documentation and warn stream wrappers developers to implement the menu.

Regarding remote file storages. I believe image styles functionality will work correctly if the wrappers correctly implement directory creation functionality. Of course, it may be impossible for some storages and a kind of creative simulation may be realized.

Can someone document the steps to reproduce this based on #20, or create an automated test demonstrating the bug?

1. Install a module that implements stream wrapper with type STREAM_WRAPPERS_WRITE_VISIBLE, and uses directory path that is not equal to public stream directory path (see sample wrapper below)
2. Attach field_image to an entity that is fieldable and has configuration form
3. Configure field instance to use this new wrapper (you sould see it in the radio button list)
4. Go to entity form and upload an image

You will see broken thumbnail immediately, and you can check that the image is where it should be (in sites/all/files/samle), but there is no appropriate directories below it.

// In .inc file
class SampleLocalStreamWrapper extends DrupalLocalStreamWrapper {

    public function getExternalUrl()
    {
        $path = str_replace('\\', '/', $this->getTarget());
        return $GLOBALS['base_url'] . '/' . self::getDirectoryPath() . '/' . drupal_encode_path($path);
    }

    function getDirectoryPath()
    {
        $path = 'sites/all/files/sample';
        return trim($path, '/');
    }
}

// And in a module
function sample_stream_wrappers() {
    return array(
        'sample' => array(
            'name' => t('Sample Files'),
            'class' => 'SampleLocalStreamWrapper',
            'description' => t('Sample local files.'),
            'type' => STREAM_WRAPPERS_WRITE_VISIBLE
        )
    );
}

Thanks @tms320c.

Now let's roll #22 into an automated test demonstrating the bug. Create a hidden test module from the snippets above, and then add a test class that installs that test module and follows the STR.

This could probably just go in to the image_module_test module that's in modules/image/tests/ instead of having to create a new module.

Alright, here's a test. Are we sure this was fixed?
The files are correctly uploaded to the stream wrapper's directory, but the image style is not created.

Since the OP and last patch, image_style_url has changed, the stream wrappers have gone PSR-0, and the default value for 'type' is hook_stream_wrappers is STREAM_WRAPPERS_NORMAL which is identical to STREAM_WRAPPERS_WRITE_VISIBLE.

Also setting $config_directory_name in settings.php does not work when path is absolute. Probably there's some issue about

Thanks @tim.plunkett.

I've created a related patch where I took a different approach to figuring out where to store image derivatives: #1821166: Support image style derivatives using a different file scheme than the original image

@tim.plunkett, Reading you comment from #27 I understand that this patch is no more actual. Should I close the issue or move it back to D7?

This is not an issue in D8. Drupal 8 is able to handle sources passed as custom stream wrappers but is buggy when it comes to to read-only stream wrappers. For the D8 read-only wrappers I've opened #2770761: Derivatives should not be created on read-only stream wrappers. This I will move back to Drupal 7 to be fixed there, if case.