| Message ID | 1816aee4f80.1026d4b311254470.8507588530121880177@elijahpepe.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ebb4a170c024 |
| Headers | show |
| Series | python: remove IOError in certain cases | expand |
Elijah Conners <business@elijahpepe.com> writes: > In certain cases, IOError caused the much more general exception OSError > to be unreachable. > > Signed-off-by: Elijah Conners <business@elijahpepe.com> Could you please provide more details about the certain cases, preferably with a reproducer? Thanks, Petr > --- > python/semanage/semanage | 7 ++----- > sandbox/sandbox | 2 -- > 2 files changed, 2 insertions(+), 7 deletions(-) > > diff --git a/python/semanage/semanage b/python/semanage/semanage > index 1d828128..c7a35fe4 100644 > --- a/python/semanage/semanage > +++ b/python/semanage/semanage > @@ -970,8 +970,8 @@ def do_parser(): > devnull = os.open(os.devnull, os.O_WRONLY) > os.dup2(devnull, sys.stdout.fileno()) > sys.exit(1) > - except IOError as e: > - sys.stderr.write("%s: %s\n" % (e.__class__.__name__, str(e))) > + except OSError as e: > + sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[1])) > sys.exit(1) > except KeyboardInterrupt: > sys.exit(0) > @@ -981,9 +981,6 @@ def do_parser(): > except KeyError as e: > sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[0])) > sys.exit(1) > - except OSError as e: > - sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[1])) > - sys.exit(1) > except RuntimeError as e: > sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[0])) > sys.exit(1) > diff --git a/sandbox/sandbox b/sandbox/sandbox > index cd5709fb..1c9379ef 100644 > --- a/sandbox/sandbox > +++ b/sandbox/sandbox > @@ -533,8 +533,6 @@ if __name__ == '__main__': > error_exit(error.args[0]) > except KeyError as error: > error_exit(_("Invalid value %s") % error.args[0]) > - except IOError as error: > - error_exit(error) > except KeyboardInterrupt: > rc = 0 > > -- > 2.29.2.windows.2
On Mon, 20 Jun 2022 11:04:17 -0700 Petr Lautrbach <plautrba@redhat.com> wrote > Could you please provide more details about the certain cases, > preferably with a reproducer? Yes, I can. In this patch, I change two files: python/semanage/semanage and sandbox/sandbox In sandbox/sandbox, IOError is unreachable as OSError always takes precedence, so it serves as useless code. The ambiguous nature of IOError and OSError, despite both serving the same purpose, is why I've submitted this patch. To reproduce, if the Sandbox() function were to be called, and an IOError occurred, OSError would handle the error_exit, not IOError (which is fine enough, since both exceptions lead to the same result, but IOError is redundant here). On the contrary, if an OSError exception occurred in the createCommandParser() function in python/semanage/semanage file while attempting to call do_parser(), since IOError is an alias of OSError in 3.3, the IOError exception would actually take precedence over the OSError exception. I'm not entirely sure what version SELinux is attempting to target, but the try except block in do_parser() is ambiguous and its implementation should be reconsidered. In that file, I've had OSError directly handle the exception. This, however, does change this function a little bit; the second argument will be displayed as an error, not the error itself. This might need to be changed. Thanks, Elijah
Elijah Conners <business@elijahpepe.com> writes: > In certain cases, IOError caused the much more general exception OSError > to be unreachable. > > Signed-off-by: Elijah Conners <business@elijahpepe.com> Acked-by: Petr Lautrbach <plautrba@redhat.com> > --- > python/semanage/semanage | 7 ++----- > sandbox/sandbox | 2 -- > 2 files changed, 2 insertions(+), 7 deletions(-) > > diff --git a/python/semanage/semanage b/python/semanage/semanage > index 1d828128..c7a35fe4 100644 > --- a/python/semanage/semanage > +++ b/python/semanage/semanage > @@ -970,8 +970,8 @@ def do_parser(): > devnull = os.open(os.devnull, os.O_WRONLY) > os.dup2(devnull, sys.stdout.fileno()) > sys.exit(1) > - except IOError as e: > - sys.stderr.write("%s: %s\n" % (e.__class__.__name__, str(e))) > + except OSError as e: > + sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[1])) > sys.exit(1) > except KeyboardInterrupt: > sys.exit(0) > @@ -981,9 +981,6 @@ def do_parser(): > except KeyError as e: > sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[0])) > sys.exit(1) > - except OSError as e: > - sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[1])) > - sys.exit(1) > except RuntimeError as e: > sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[0])) > sys.exit(1) > diff --git a/sandbox/sandbox b/sandbox/sandbox > index cd5709fb..1c9379ef 100644 > --- a/sandbox/sandbox > +++ b/sandbox/sandbox > @@ -533,8 +533,6 @@ if __name__ == '__main__': > error_exit(error.args[0]) > except KeyError as error: > error_exit(_("Invalid value %s") % error.args[0]) > - except IOError as error: > - error_exit(error) > except KeyboardInterrupt: > rc = 0 > > -- > 2.29.2.windows.2
Petr Lautrbach <plautrba@redhat.com> writes: > Elijah Conners <business@elijahpepe.com> writes: > >> In certain cases, IOError caused the much more general exception OSError >> to be unreachable. >> >> Signed-off-by: Elijah Conners <business@elijahpepe.com> > > Acked-by: Petr Lautrbach <plautrba@redhat.com> > Merged, thanks! >> --- >> python/semanage/semanage | 7 ++----- >> sandbox/sandbox | 2 -- >> 2 files changed, 2 insertions(+), 7 deletions(-) >> >> diff --git a/python/semanage/semanage b/python/semanage/semanage >> index 1d828128..c7a35fe4 100644 >> --- a/python/semanage/semanage >> +++ b/python/semanage/semanage >> @@ -970,8 +970,8 @@ def do_parser(): >> devnull = os.open(os.devnull, os.O_WRONLY) >> os.dup2(devnull, sys.stdout.fileno()) >> sys.exit(1) >> - except IOError as e: >> - sys.stderr.write("%s: %s\n" % (e.__class__.__name__, str(e))) >> + except OSError as e: >> + sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[1])) >> sys.exit(1) >> except KeyboardInterrupt: >> sys.exit(0) >> @@ -981,9 +981,6 @@ def do_parser(): >> except KeyError as e: >> sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[0])) >> sys.exit(1) >> - except OSError as e: >> - sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[1])) >> - sys.exit(1) >> except RuntimeError as e: >> sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[0])) >> sys.exit(1) >> diff --git a/sandbox/sandbox b/sandbox/sandbox >> index cd5709fb..1c9379ef 100644 >> --- a/sandbox/sandbox >> +++ b/sandbox/sandbox >> @@ -533,8 +533,6 @@ if __name__ == '__main__': >> error_exit(error.args[0]) >> except KeyError as error: >> error_exit(_("Invalid value %s") % error.args[0]) >> - except IOError as error: >> - error_exit(error) >> except KeyboardInterrupt: >> rc = 0 >> >> -- >> 2.29.2.windows.2
diff --git a/python/semanage/semanage b/python/semanage/semanage index 1d828128..c7a35fe4 100644 --- a/python/semanage/semanage +++ b/python/semanage/semanage @@ -970,8 +970,8 @@ def do_parser(): devnull = os.open(os.devnull, os.O_WRONLY) os.dup2(devnull, sys.stdout.fileno()) sys.exit(1) - except IOError as e: - sys.stderr.write("%s: %s\n" % (e.__class__.__name__, str(e))) + except OSError as e: + sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[1])) sys.exit(1) except KeyboardInterrupt: sys.exit(0) @@ -981,9 +981,6 @@ def do_parser(): except KeyError as e: sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[0])) sys.exit(1) - except OSError as e: - sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[1])) - sys.exit(1) except RuntimeError as e: sys.stderr.write("%s: %s\n" % (e.__class__.__name__, e.args[0])) sys.exit(1) diff --git a/sandbox/sandbox b/sandbox/sandbox index cd5709fb..1c9379ef 100644 --- a/sandbox/sandbox +++ b/sandbox/sandbox @@ -533,8 +533,6 @@ if __name__ == '__main__': error_exit(error.args[0]) except KeyError as error: error_exit(_("Invalid value %s") % error.args[0]) - except IOError as error: - error_exit(error) except KeyboardInterrupt: rc = 0
In certain cases, IOError caused the much more general exception OSError to be unreachable. Signed-off-by: Elijah Conners <business@elijahpepe.com> --- python/semanage/semanage | 7 ++----- sandbox/sandbox | 2 -- 2 files changed, 2 insertions(+), 7 deletions(-)