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

fileUpload: Unable to upload a file without extension #4863

Closed
stolp opened this issue Jun 6, 2019 · 10 comments
Closed

fileUpload: Unable to upload a file without extension #4863

stolp opened this issue Jun 6, 2019 · 10 comments
Labels
6.2.26 7.0.9 🐞 defect Bug...Something isn't working
Milestone

Comments

@stolp
Copy link
Contributor

stolp commented Jun 6, 2019

1) Environment

  • PrimeFaces version: 7.0.3
  • Does it work on the newest released PrimeFaces version? No
  • Affected browsers: All

2) Expected behavior

It should be possible to upload any existing file.

3) Actual behavior

Files without extensions cause a FacesExeption.

4) Steps to reproduce

Create a file named 'foobar' and try to upload it

Reason for this is the following code in FileUploadUtils.getValidFilename:

        String extension = FilenameUtils.EXTENSION_SEPARATOR_STR + FilenameUtils.getExtension(filename);

        if (extension.equals(FilenameUtils.EXTENSION_SEPARATOR_STR)) {
            throw new FacesException("File must have an extension");
        }

This seems to enforce the presence of a filename extension for later checks. I am unsure if this is really needed.

The most pressing problems are:

  • A user of a primefaces based application is not aware that this restriction exists
  • Any primefaces based application is not able to catch that exception properly
@tandraschko tandraschko added the 🐞 defect Bug...Something isn't working label Jun 7, 2019
@tandraschko
Copy link
Member

@cnsgithub can you assist?

@Rapster
Copy link
Member

Rapster commented Jun 25, 2019

Indeed, no extension should be allowed. After having a look to FileUploadUtils, I think @cnsgithub mentionned https://github.com/augustd/owasp-java-fileio. It looks like there are all the utils we need here.

@hfluz
Copy link
Contributor

hfluz commented Sep 1, 2019

One of my users reported this issue recently. It seems there not much I can do in my application, since some users don't even understand about file extensions and that sometimes their files don't have one. I didn't find a way to better handle the exception, it would be nice if it could at least return a validation message.

I thought about trying to write a PR, but I as it was already mentioned, the extension is used in other methods of FileUploadUtils and it might cause some side effects.

@melloware
Copy link
Member

I think we should throw an exception. i know its used in a few place but if a file name is not valid we should let end users know.

@stolp
Copy link
Contributor Author

stolp commented Sep 2, 2019

I think you got this the wrong way.

There is absolutely no reason to not accept a filename without an extension for upload. It is perfectly valid, and especially common on Unix systems (README, Makefile etc.).

So I think an throwing an exception is the completely wrong solution here.

@tandraschko
Copy link
Member

@stolp +1

@melloware
Copy link
Member

@stolp there are two different issues here.

  1. The fact that the FileUpload validator silently eats exception when a file name is bad is a bad user experience.

  2. Correcting the ability to allow an extensionless file.

I think both issues need to be fixed.

@melloware
Copy link
Member

Sorry as these 2 issues are related: #4843

@stolp
Copy link
Contributor Author

stolp commented Sep 2, 2019

@melloware I agree, the way exceptions are thrown and handled here is broken. Sorry for misunderstanding you here.

But in case of the issue here, the FacesExceptions thrown in getValidFilename should be omitted (as in the case of the empty extension) or at least be catched properly elsewhere in the component code (for the other cases present there).

@melloware
Copy link
Member

Yep I am going to attempt to fix both issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.2.26 7.0.9 🐞 defect Bug...Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants