jonhadfield / python-hosts

a hosts file manager library written in python

MIT License

125 stars 33 forks source link

fix: raise UnableToWriteHosts for privileged users #35

Closed lucasfcnunes closed 3 years ago

lucasfcnunes commented 3 years ago

even when sudo/root editing (file = 644) it was raising this exception. simple change in edit mode w -> r+ can solve this for privileged users.

Fixes #21

lucasfcnunes commented 3 years ago

maybe adding a "mode=" parameter in .write would be better... idk

coveralls commented 3 years ago

Coverage remained the same at 0.0% when pulling 40b4332d0c18bbbbe01727505958aecb9115b563 on lucasfcnunes:devel into f9210dda91d2991435d06635760932cb63357df1 on jonhadfield:devel.

coveralls commented 3 years ago

Coverage remained the same at 0.0% when pulling bb7993bb06739848879cda42a1ecfb2f4882b51c on lucasfcnunes:devel into f9210dda91d2991435d06635760932cb63357df1 on jonhadfield:devel.

jonhadfield commented 3 years ago

Thanks for the PR. I appreciate you taking the time to look at this issue. Would you be able separate this into two PRs (one for the file write issue, and the second for comments)? It just keeps it cleaner being able to comment and work on.

I've had a look at the write issue and the +r generates a write exception on Linux, and MacOS. I also tried on Windows 10 (64-bit) and use of +r also results in an error. I suspect the issue relates to this SO post and is related to 32-bit Windows only. Please could you confirm the OS (inc. architecture) and version of python you are testing this on?

lucasfcnunes commented 3 years ago

*r+ opens for reading and writing (can't truncate the file)

I'm currently using this project in Linux Ubuntu (20.04, cpython 3.8) and a Alpine Docker (python:3-alpine, cpython 3.9) container as root. it was throwing the unable to write exception and this fixed for my use case.

probably the safe route is to create a new parameter "mode" in .write... i'll do that!

about the inline comments PR, i thought i hadn't pulled that: it was a mistake, sorry :(

codecov-commenter commented 3 years ago

Codecov Report

Merging #35 (40b4332) into devel (f9210dd) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             devel       #35   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          321       321           
=========================================
  Hits           321       321

Impacted Files	Coverage Δ
python_hosts/hosts.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f9210dd...40b4332. Read the comment docs.