Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filename encoding inconsistent with stapler #41

Open
iredmedia opened this issue Apr 27, 2019 · 10 comments
Open

Filename encoding inconsistent with stapler #41

iredmedia opened this issue Apr 27, 2019 · 10 comments

Comments

@iredmedia
Copy link

I'm migrating from stapler and my last hurdle is that filenames are being encoded differently.

Stapler would save the filename without encoding.

As a work around i've extended the Interpolator to override the filename function, doing a urldecode on $attachment->originalFilename().

Would love some feedback, is this something you'd like to have configurable? Is there a better solution? Would be happy to contrib.

@czim
Copy link
Owner

czim commented Apr 28, 2019

I don't understand why it should be different. I've been looking at the code, and the logic is the same as far as I can see:

Stapler and Paperclip share exactly this same logic:

  • Interpolator::filename() calls directly on Attachment:originalFilename() without any modification.
  • originalFilename is $this->instance->getAttribute("{$this->name}_file_name").

And both use :filename in the default setup (for S3 in the case of Stapler, anyway).

So the most help I need is with understanding this.

If a short-term workaround is needed, I'd prefer adding something like a :url_decoded_filename placeholder to the interpolator for this purpose.

@iredmedia
Copy link
Author

Fascinating, I've logged out all the way back to symfony/http-foundation and I'm seeing that it's not encoding the filename.

laravel/framework @ v5.5.45
codesleeve/laravel-stapler @ v1.0.09
codesleeve/stapler @ v1.2.0
symfony/http-foundation @ v3.4.26

vendor/symfony/http-foundation/File/UploadedFile.php

    public function getClientOriginalName()
    {
        \Log::info('getClientOriginalName', [$this->originalName]); // My log
        return $this->originalName;
    }

@czim
Copy link
Owner

czim commented Apr 29, 2019

Perhaps it's something to do with the database the filename is stored to? This too should be 100% identical between Stapler and Paperclip, but it might be worth examining.

@iredmedia
Copy link
Author

iredmedia commented Apr 30, 2019

I'm loosing my mind over this issue. The problem has apparently disappeared (even though I tested multiple times). Closing issue.

@czim
Copy link
Owner

czim commented May 1, 2019

Alright, those are the fun ones! Let me know if it comes back to stay. 👍

@iredmedia
Copy link
Author

Hey there!

Finally had time to take another look at this. It seems that the characters that cause paperclip to fail (Laravel 5.8 + Postgres + S3)

The characters that are causing us to fail (without thinking about stapler, just with a fresh paperclip install are: +, ?, and "

Upon uploading a file, say +.png, the result returned is somepath/+.png. This fails, however checking somepath/%2B.png works fine.

Any advice/insights on this? Stapler didn't suffer from this issue.

@czim
Copy link
Owner

czim commented Jul 21, 2019

What what settings do you use for S3 storage? I've been seeing some issues with S3 in some cases..

@czim czim reopened this Jul 21, 2019
@czim
Copy link
Owner

czim commented Jul 28, 2019

Update: I've been experimenting with Minio, trying to get a setup that simulates S3 well enough for automated testing. Unfortunately it does not appear that the behavior is identical. If anyone gets this working (or rather: gets it to break in the same way), I'd be obliged to hear the details!

@austenc
Copy link
Contributor

austenc commented Jun 22, 2020

We just noticed this with filenames containing # too.

@austenc
Copy link
Contributor

austenc commented Jun 22, 2020

So it seems to be happening with characters that are otherwise valid in a URI. Particularly +, ? and #.

Other characters (such as a space) seem to be automatically encoded by modern browsers to things like %20 -- but since characters such as # could be part of a URL, they aren't encoded automatically. Seems like paperclip or czim/file-handling should account for some of these characters in the filename when generating the URL. Happy to submit a PR, but not sure yet where to make this change 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants