glide icon indicating copy to clipboard operation
glide copied to clipboard

Should filter $params against available manipulations/params

Open midweste opened this issue 3 years ago • 2 comments

Great package! I'm building a wordpress implementation of glide (will share when finished) and I've found what I consider is a slight bug in the handling of query string manipulations.

I think params should be filtered by defined params because any arbitrary query string variables will create new images

<img src="/img/users/myimage.jpg?w=300&h=400&fit=crop">
<img src="/img/users/myimage.jpg?w=300&h=400&fit=crop&asdf=asdf">
<img src="/img/users/myimage.jpg?w=300&h=400&fit=crop&qwer=qwer">

Will all create new images, technically they should be the same image as the additional query string variables do not constitute a new variant. Currently three cache images are created and I'm assuming that has to do with the name of cache files being a hash of the parameters.

Just a quick couple quick functions/methods for filtering below (minimally tested). I haven't got into the source yet to see where this would need to go, but when I do I'll try and put in a pull request.


function getQueryStringManipulationsAllowed(): array
{
    // https://glide.thephpleague.com/2.0/api/quick-reference/
    return [
        'or',
        'flip',
        'crop',
        'w',
        'h',
        'fit',
        'dpr',
        'bri',
        'con',
        'gam',
        'sharp',
        'blur',
        'pixel',
        'filt',
        'mark',
        'markw',
        'markh',
        'markx',
        'marky',
        'markpad',
        'markpos',
        'markalpha',
        'bg',
        'border',
        'q',
        'fm',
    ];
}

// this method would need any other qs variables accepted!!
function getQueryStringOther(): array
{
    return [
        'p', // preset
        's', // security signature
    ];
}

function getQueryStringFiltered(array $params): array
{
    $merged = array_merge(getQueryStringManipulationsAllowed(), getQueryStringOther());
    return array_intersect_key($params, array_flip($merged));
}


midweste avatar Oct 29 '21 16:10 midweste

@midweste Still interested in making that pull request? :slightly_smiling_face:

ADmad avatar Jan 04 '22 16:01 ADmad

@ADmad Yes! Will try to put one in this week :)

midweste avatar Feb 23 '22 14:02 midweste