https://bugzilla.redhat.com/show_bug.cgi?id=1202604
--- Comment #10 from Zbigniew Jędrzejewski-Szmek <zbyszek(a)in.waw.pl> ---
(In reply to Stephen Gallagher from comment #9)
(In reply to Zbigniew Jędrzejewski-Szmek from comment #6)
> It seems that tempfile.NamedTemporaryFile would do the right thing without
> requiring all this manual cleanup:
>
The reason I'm using the tempfile.mkstemp() implementation is this:
"Creates a temporary file in the most secure manner possible. There are no
race conditions in the file’s creation, assuming that the platform properly
implements the os.O_EXCL flag for os.open()."
TemporaryFile and
NamedTemporaryFile and mkstemp share the implementation, so
there's no difference in security, only in the way that file is automatically
deleted.
(In reply to Stephen Gallagher from comment #8)
Well, I was trying to address a potential containerized case where
either
python release might be available. If I was only going to include the script
for the default python, it doesn't make sense to build both (this module
isn't really intended to be imported by other scripts, though it could be).
So if you're building a Dockerfile and want the image to generate a
certificate on startup, you probably want to be able to use whichever
version of the script matches the version of python already in the
environment.
But this actually makes things more complicated for the user, because
if the
have only the non-default version installed, there's no /usr/bin/sscg. So maybe
there shouldn't be packages for both versions, just one, always providing
/usr/bin/sscg, and internally it should use either python2 or python3,
whichever one is the default for this Fedora version.
> In the python code, a bare try: ... except: (without any
exception na
> after "except") is used. In some places the exception is re-raised, but
in
> others not. This is a problem because the script might be ended with ^C (or
> a signal) and silently "swallow" the exception. The script should always
> exit after being interrupted. It also should not print an error message like
> "Failed to write..." when killed.
Ah, where did I do that? It wasn't intentional.
I'm not sure about what one
specifically you're asking... "bare" except is used
almost everywhere.
In src/sscg/main.py there is this:
try:
os.unlink(file)
except:
# Nothing we can do if we get an error
pass
> It seems that gettext is called on formatted arguments:
> _("Could not write to {0}. Error: {1}".format(...))
> but it should be before:
> _("Could not write to {0}. Error: {1}").format(...)
>
Typo, thanks for catching it.
> Why open the temp file in text mode, then decode and re-encode the
> certificate. Maybe it would be simpler to open it in binary mode and skip
> the decode step?
>
I don't do I? I thought the default was binary mode, which is why I was
decoding the UTF-8 to the binary representation. Or have I confused myself?
(I don't like dealing with encodings...)
Yes, binary is the default. But
decoding goes from bytes to unicode (i.e. text,
i.e. str in Python 3). Encoding goes from text (i.e. str) to a specific
encoding, for example UTF-8, and the bytes type. So you were writing text to a
binary file. This actually does not work in Python 3:
>> f = tempfile.NamedTemporaryFile()
>> f.write('blah')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib64/python3.4/tempfile.py", line 399, in func_wrapper
return func(*args, **kwargs)
TypeError: 'str' does not support the buffer interface
although it does work under Python 2.
> It seems that this will be usable a library. I think that
calling sys.exit()
> should not be done, except in main. Errors should not be printed either.
> Simply raising an exception with an appropriately formatted message and
> letting the caller handle it would nicely simplify things.
It's not supposed to be used directly as a library, only as a script.
OK.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component