Bugs with Image input type

[hrabovec (Juraj Hrabovec)]

@mhamstra

Correct - that’s basically the first thing I do in a typical site, configuring specific sources for specific uses, but I suppose not everyone does that with a new source.

I would consider this a very specific setup, we never needed media source from web root. It might be convenient for admin. But other users must not have access to it. We only create specific sources for editors.

@mhamstra

That’s basically how it was before - we were passing the url and either the fields’ source or the contentblocks.image.source system setting value. But then apparently that stopped working for whatever reason…

Maybe it never really worked, just nobody noticed. You didn’t because of your webroot default media source. We didn’t either because for years and many sites always using image crops, which works fine. This is first time page design let users decide if they want crop.

@mhamstra

I guess we’re going to have to build our own phpthumb endpoint that tries to parse out the base url of whatever media source and hope for the best. Don’t think we can do that client-side which is where most of this logic is at.

To me it seems that all you need to do is:

• return source parameter back to phpthumb call
• if basePathRelative and url starts with MODX_BASE_PATH, strip that path
• if url starts with basePath, strip that path
• call phpthumb with this relative path and source number

Basically, you need to do inverse logic to prepareSrcForThumb($src) function:

            if (strpos($src, DIRECTORY_SEPARATOR) !== 0) {
                $src = !empty($properties['basePath']) ? $properties['basePath'] . $src : $src;
                if (!empty($properties['basePathRelative'])) {
                    $src = $this->ctx->getOption('base_path', null, MODX_BASE_PATH) . $src;
                }
            }

Or you can call phpthumb directly like phpthumof does, without any MediaSource stuff. But not sure what happens with S3 sources.

[hrabovec (Juraj Hrabovec)]

@mhamstra

This is very broken, conceptually that seems like it’s supposed to be checking for absolute files and let those through, but it only let’s an empty string through - meaning it just checks /.

Yes, this part of code probably never runs and is probably not needed.

Looking few lines above I wander what would happen if somebody starts regular file name with ‘http’?
if (substr($src, 0, 4) != 'http')
This is very poor check for http protocol. Should be someting like
if (substr($src, 0, 7) != 'http://' && substr($src, 0, 8) != 'https://')
or some regex.