Throw exception on empty filepath in create_folder

[MVrachev]

Fixes issue #: #241

Description of the changes being introduced by the pull request:

Now, when securesystemslib.storage.FilesystemBackend.create_folder()
is called with an empty string it returns immediately,
without exception, but if we call os.mkdir() with an empty string
we receive this exception:
"[Errno 2] No such file or directory: ''

If we want to mimic the os.mkdir() function when using the
securesystemslib.storage.FilesystemBackend.create_folder()
the function we need to throw our respective exception
securesystemslib.exceptions.StorageError with an appropriate message
when an empty path is given.

Please verify and check that the pull request fulfils the following
requirements:

• The code follows the Code Style Guidelines
• Tests have been added for the bug fix or new feature
• Docs have been added for the bug fix or new feature

[coveralls]

Coverage remained the same at 98.944% when pulling adf5d39 on MVrachev:fix-failing-tuf-tests into 22a3c4e on secure-systems-lab:master.

[joshuagl]

Thanks for this submission Martin! I think we can make create_folder() more Pythonic by removing the check for an empty string and instead refining our exception handler. What do you think?

[joshuagl]

I'd rather we deleted this conditional entirely and instead improve the exception handler in try/except block around the os.makedirs() call below.

On Python2 os.makedirs() with an empty string throws an OSError, whereas on Python3 we'll get a FileNotFoundError. In either case the exception's errno field will equal errno.ENOENT.

We should be able to adapt our existing exception handling to check for errno.ENOENT and pass a different message when raising securesystemslib.exceptions.StorageError in that case.

[MVrachev]

Thanks for this submission Martin! I think we can make create_folder() more Pythonic by removing the check for an empty string and instead refining our exception handler. What do you think?

Yep, it does make sense given that we are already doing exception handling.
I decided to add an additional check if filepath is empty string together with errno.ENOENT because I am not sure if there could be other cases where errno.ENOENT can be thrown besides empty string and thus we could raise an exception with wrong message.

[joshuagl]

I decided to add an additional check if filepath is empty string together with errno.ENOENT because I am not sure if there could be other cases where errno.ENOENT can be thrown besides empty string and thus we could raise an exception with wrong message.

Statements like this make me nervous. We shouldn't be adding code "just in case", so I decided to reason about under what other conditions we might get an ENOENT:

In [7]: def mkdir(path):
   ...:     try:
   ...:         os.mkdir(path)
   ...:     except OSError as err:
   ...:         if err.errno == errno.ENOENT and not path:
   ...:             print('Empty path')
   ...:         elif err.errno == errno.ENOENT:
   ...:             print('ENOENT')
   ...:         else:
   ...:             print('some other error')
   ...:

In [8]: mkdir('')
Empty path

In [9]: mkdir('/Users/jlock/FooFoo/bar')
ENOENT

In [10]: mkdir('/System/hacks')
some other error

We get an ENOENT for an empty string and also for when some parent components of the path do not exist, i.e I don't have a directory named FooFoo in my home directory, so os.mkdir() can't create a child directory bar there.
Therefore, if we want to show an error message for an empty string we do need this additional logic.

[joshuagl]

Thanks for the fix @MVrachev

[MVrachev]

We get an ENOENT for an empty string and also for when some parent components of the path do not exist, i.e I don't have a directory named FooFoo in my home directory, so os.mkdir() can't create a child directory bar there.

Because there is another case where we can receive ENOENT different from passing an empty string, I thought it's a good idea to add that check.
The exception message itself is specific that the problem is exactly that filepath is an empty string and that's why I wanted to make sure that we throw it only when that occurs.