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

Fix exception on iOS in _get_localzone #895

Closed
wants to merge 1 commit into from

Conversation

gio3k
Copy link

@gio3k gio3k commented Jul 29, 2022

On iOS the macOS specific part of the function just instantly throws an exception due to no systemsetup file being found

This PR just wraps that part in a try block and continues on a FileNotFoundError

iOS doesn't come with systemsetup so we need to watch for a FileNotFoundError
gio3k added a commit to gio3k/DarwinPrint that referenced this pull request Aug 29, 2022
Comment on lines +68 to +82
try:
c = subprocess.Popen(['systemsetup', '-gettimezone'],
stdout=subprocess.PIPE)
sys_result = c.communicate()[0]
c.wait()
tz_match = _systemconfig_tz.search(sys_result)
if tz_match is not None:
zone_name = tz_match.group(1)
try:
return pytz.timezone(zone_name)
except pytz.UnknownTimeZoneError:
pass
# iOS doesn't come with systemsetup
except FileNotFoundError:
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of having a huge try-block, you can make it more specific moving the non-critical things to the else-block of a try-except:

Suggested change
try:
c = subprocess.Popen(['systemsetup', '-gettimezone'],
stdout=subprocess.PIPE)
sys_result = c.communicate()[0]
c.wait()
tz_match = _systemconfig_tz.search(sys_result)
if tz_match is not None:
zone_name = tz_match.group(1)
try:
return pytz.timezone(zone_name)
except pytz.UnknownTimeZoneError:
pass
# iOS doesn't come with systemsetup
except FileNotFoundError:
pass
try:
c = subprocess.Popen(['systemsetup', '-gettimezone'],
stdout=subprocess.PIPE)
# iOS doesn't come with systemsetup
except FileNotFoundError:
pass
else:
sys_result = c.communicate()[0]
c.wait()
tz_match = _systemconfig_tz.search(sys_result)
if tz_match is not None:
zone_name = tz_match.group(1)
try:
return pytz.timezone(zone_name)
except pytz.UnknownTimeZoneError:
pass

it helps identifying which part we expect to throw the exception, and avoids catching any unexpected instances of this exceptions – which probably should stay unhandled so that the bug becomes visible and one gets a nice backtrace.

(not a maintainer, just randomly browsed through PRs)

akx added a commit that referenced this pull request Oct 31, 2022
According to https://truesecdev.wordpress.com/2015/04/09/hidden-backdoor-api-to-root-privileges-in-apple-os-x/comment-page-1/
the `systemsetup` command has required superuser privileges since
macOS 10.8.5 (which has been EOL for over 6 years at the time of writing).
We shouldn't expect to use a codepath that requires a helper tool
that requires su in any regular use; IOW, _if_ a Babel user had ever
reached this path without having been superuser, it would have failed anyway
on any currently supported version of macOS.

Closes #895 (supersedes it).
@akx akx closed this in 03c8fae Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants