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

Add check for directory name #726

Closed
wants to merge 1 commit into from
Closed

Conversation

Akasurde
Copy link
Member

Fix adds check to verify if user provided input is not
a directory when filename is required.

Fixes: https://pagure.io/freeipa/issue/6883

Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ipalib/util.py Outdated

if os.path.isdir(filename):
raise errors.FileError(reason=_('Directory name found instead of '
'Filename'))
Copy link
Contributor

Choose a reason for hiding this comment

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

The error does not say much and has F in "filename" uppercased for some reason, could you rather go with something like "Expected file but {fname} is a directory".format(fname=filename)?

Copy link
Member Author

Choose a reason for hiding this comment

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

OK.

Copy link
Contributor

Choose a reason for hiding this comment

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

It must be %(fname)s translation strings doesn't support format (in versions we need at least)

Copy link
Contributor

Choose a reason for hiding this comment

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

I keep forgetting that, thank you.

ipalib/util.py Outdated

if os.path.isdir(filename):
raise errors.FileError(reason=_('Expected file but {fname} is a '
'directory'.format(fname=filename))
Copy link
Contributor

Choose a reason for hiding this comment

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

You are missing a parentheses.

@tiran
Copy link
Member

tiran commented Apr 24, 2017

What about other types that might cause trouble, e.g. socket, fifo, device files, dangling symlinks?

@Akasurde
Copy link
Member Author

@tiran Do you think we should allow only files here ?

Fix adds check to verify if user provided input is not
a directory when filename is required.

Fixes: https://pagure.io/freeipa/issue/6883

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
@stlaz
Copy link
Contributor

stlaz commented May 4, 2017

Obviously we can't push this until the tests pass.

@Akasurde Akasurde closed this May 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants