Skip to content

Add encrypt_mimes config option #88

Merged
pxpm merged 4 commits into
mainfrom
fix-mime-type-tampering-protection
Mar 26, 2026
Merged

Add encrypt_mimes config option #88
pxpm merged 4 commits into
mainfrom
fix-mime-type-tampering-protection

Conversation

@pxpm
Copy link
Copy Markdown
Contributor

@pxpm pxpm commented Mar 25, 2026

This allow developers to disable the encrypt/decrypt of mime types.

IMO, it shouldn't hurt having this little obfuscation, but for some developers they might prefer to have the clean mime types and that's ok too.

Copy link
Copy Markdown
Contributor

@jnoordsij jnoordsij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pxpm for the quick fix! I think the disable flag is nice, especially for those willing to override or call the route from some other view of their own. I've tested this locally and it seems to work great for me.

Just for reference, that is also my use case: I have a custom HTML editor field, where I manually call the elFinder popup for integration, then pass along any elFinder option in JS from there. With this config flag I no longer need to 'hack' and add Crypt::encrypt('') as a parameter just to consume the view.

Edit: I've left some small suggestion for an IMO cleaner way to generate the URL that I use.

Comment on lines 3 to +7
$field['attributes']['data-elfinder-trigger-url'] = $field['attributes']['data-elfinder-trigger-url'] ?? url(config('elfinder.route.prefix').'/popup/'.$field['name']);
$field['attributes']['data-elfinder-trigger-url'] .= '?mimes='.urlencode(Crypt::encrypt($field['mime_types'] ?? ''));
$mimeTypes = $field['mime_types'] ?? '';
$field['attributes']['data-elfinder-trigger-url'] .= '?mimes='.urlencode(
config('elfinder.encrypt_mimes', true) ? Crypt::encrypt($mimeTypes) : json_encode($mimeTypes, JSON_UNESCAPED_SLASHES)
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally use the route helper here with array parameters, which I both prefer for readability and because it neatly handles URL parsing for both 'required' and 'optional' parameters.

$mimeTypes = $field['mime_types'] ?? '';
$field['attributes']['data-elfinder-trigger-url'] ??= route(
    'elfinder.popup',
    [
        'input_id' => $field['name'],
        'mimes' => config('elfinder.encrypt_mimes', true) ? Crypt::encrypt($mimeTypes) : json_encode($mimeTypes, JSON_UNESCAPED_SLASHES),
    ],
);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's cleaner, but at this point we can't be sure if people are using elfinder.popup as their url, if they changed it or not.
since there is nothing "wrong" per se as it is now, I think I would rather keep it for now 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be fair I would be slightly more worried about the actual url changing than the alias (alias comes from https://github.com/barryvdh/laravel-elfinder/blob/30700116a94828202b777da33a6d5c1ee7d004a6/src/ElfinderServiceProvider.php#L59). But both work fine, so perfectly fine to skip this.

Thanks for merging 🥂

$field['wrapper']['data-elfinder-trigger-url'] = $field['wrapper']['data-elfinder-trigger-url'] ?? url(config('elfinder.route.prefix').'/popup/'.$field['name'].'?multiple=1');

$field['wrapper']['data-elfinder-trigger-url'] .= '&mimes='.urlencode(Crypt::encrypt($field['mime_types'] ?? ''));
$mimeTypes = $field['mime_types'] ?? '';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, but with multiple => 1 in the array

@pxpm pxpm merged commit 261971c into main Mar 26, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this to Done in Triage Mar 26, 2026
@pxpm pxpm deleted the fix-mime-type-tampering-protection branch March 26, 2026 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants