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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Fix crash if no read permissions for /proc/uptime, and use fallbacks instead #66

Merged
merged 9 commits into from
May 11, 2020

Conversation

ingrinder
Copy link
Collaborator

On some systems without read permissions on /proc/uptime (e.g. Android), or lacking /proc/uptime (BSD) Archey will fail with a traceback. This PR fixes this bug and adds additional fallback detections to the Uptime entry to support these systems.

Description

Reworked the uptime entry somewhat to now try a few methods of detecting uptime, hopefully preventing complete failure with a traceback.
The uptime is now found as follows:

  • Try reading /proc/uptime
  • Try using the clocks provided by Python's time module for Linux, macOS and BSD.
  • Try parsing the uptime command (from procps(-ng)

Added test cases for this new behaviour, as well as getting uptime output from various systems and making sure these are all handled correctly.

How has this been tested ?

Using the test cases with various different systems' behaviour, and on my local machine as usual 馃槂

Types of changes :

  • Bug fix (non-breaking change which fixes an issue)

Checklist :

  • [IF NEEDED] I have updated the test cases (which pass) accordingly ;
  • My changes looks good ;
  • I agree that my code may be modified in the future ;
  • My code follows the code style of this project (PEP8).

(I removed the unneeded checklist items so our PR progress bar is somewhat meaningful 馃槂)

@ingrinder ingrinder added bug 馃悰 A real glitch has been found enhancement 猬嗭笍 Implements a new feature, fixes or improves existing ones labels Apr 30, 2020
@HorlogeSkynet HorlogeSkynet added this to IN PROGRESS in Core May 1, 2020
This crash occured on locked-down systems, e.g. Android.

Also...
Adds support for macOS and BSD systems with this entry.
Adds fallback to the `uptime` command for systems that have it
(e.g. Android!)
Adds test cases as appropriate.
Also fix some weird formatting in the uptime entry tests.
@ingrinder
Copy link
Collaborator Author

Sorry, I didn't really think then -- I only force pushed a rebase to master, feel free to force push over it if you're working on anything on this branch (I've gotten too used to working on branches on my own 馃槙)

@HorlogeSkynet HorlogeSkynet added this to the v4.7.2 milestone May 5, 2020
@HorlogeSkynet
Copy link
Owner

Hey Michael, no problem I've not started to work on this one last WE.
I'm now reviewing this, and I'll try to DRY it a bit.

First question though, is uptime --pretty not widely available ?
Just in case, it would allow us to drastically simplify its parsing 馃憣
Bye 馃憢

@ingrinder
Copy link
Collaborator Author

Unfortunately no 馃槮 - both busybox and the BSDs accept no arguments to uptime, and throw errors if any are specified (not to mention the --arg format is invalid BSD syntax).

It may be possible to use the output of sysctl on the BSDs (free/open/macOS all seem to have the kern.boottime value) however this wouldn't help in the case of e.g. Android which is Linux-based but simply has no user read access from /proc - so I'm not sure if it's worth trying to parse sysctl if we're going to need to specify how to parse the uptime command anyway...?

Sidenote:
I've noticed we also traceback in the Model entry, CPU entry and RAM entry on the BSDs due to accessing /proc files which don't exist. Perhaps we should start another branch with these fixes to add official BSD support - the distro package already gives sensible info on it and I believe these tracebacks are the only showstoppers (plus some ASCII art 馃槃).

@ingrinder ingrinder mentioned this pull request May 6, 2020
13 tasks
@HorlogeSkynet
Copy link
Owner

HorlogeSkynet commented May 6, 2020

Thanks for the precision !
I'll try to propose you my changes very soon 馃檪

EDIT : Yes you're right, no need to play with sysctl if we end up parsing uptime anyhow for some cases 馃憣

archey/entries/uptime.py Outdated Show resolved Hide resolved
@HorlogeSkynet HorlogeSkynet self-requested a review May 9, 2020 07:16
BSD systems report seconds for < 1 minute uptime! So support has been
added to parse this.

Additionally improved readibility of our huge regex's comments.

Changed the regex's test to be far more robust, using various sensibly
chosen values, permutating them with variations of the current system
time and user/load average section of the `uptime` output. This
basically means we test our regex with over 45,000 strings :)

Localisation fix also added to `check_output` in `Uptime`.
@ingrinder
Copy link
Collaborator Author

I've made some changes to the tests so they're a lot less hotch-potch, with purposefully picked values on edge-cases etc. I've also added variations for the current time, and users/loadaverage sections taking us to many thousands of strings tested on the regex... Maybe it's extreme but it picked up on a problem when I added seconds support in the last commit 馃槂. Let me know if this is better!

@HorlogeSkynet HorlogeSkynet merged commit 584d1f3 into master May 11, 2020
Core automation moved this from IN PROGRESS to DONE May 11, 2020
@HorlogeSkynet HorlogeSkynet deleted the bugfix/uptime branch May 11, 2020 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 馃悰 A real glitch has been found enhancement 猬嗭笍 Implements a new feature, fixes or improves existing ones
Projects
Core
  
DONE
Development

Successfully merging this pull request may close these issues.

None yet

2 participants