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

Pyfa 2.8.0 not starting - yaml.FullLoader issue #1901

Closed
DeeDeeRanged opened this issue Mar 25, 2019 · 10 comments
Closed

Pyfa 2.8.0 not starting - yaml.FullLoader issue #1901

DeeDeeRanged opened this issue Mar 25, 2019 · 10 comments
Labels
bug Confirmed to be a bug fixed This issue has been fixed! Oh joy!

Comments

@DeeDeeRanged
Copy link

Bug Report

version 2.8.0:
Error in sys.excepthook:
Traceback (most recent call last):
File "/home/filip/Games/EVE/Pyfa-2.8.0/gui/errorDialog.py", line 40, in HandleException
with config.logging_setup.threadbound():
AttributeError: 'NoneType' object has no attribute 'threadbound'

Original exception was:
Traceback (most recent call last):
File "Games/EVE/Pyfa-2.8.0/pyfa.py", line 104, in
config.defPaths(options.savepath)
File "/home/filip/Games/EVE/Pyfa-2.8.0/config.py", line 116, in defPaths
data = yaml.load(file, Loader=yaml.FullLoader)
AttributeError: module 'yaml' has no attribute 'FullLoader'

Expected behavior:

Normal startup like with 2.7.0 :)

Actual behavior:

See error message

Detailed steps to reproduce:

python3 pyfa.py in terminal

Fits involved in EFT format (Edit > To Clipboard > EFT):

Release or development git branch? Please note the release version or commit hash:

stable 2.8.0

Operating system and version (eg: Windows 10, OS X 10.9, OS X 10.11, Ubuntu 16.10):

Debian buster/testing

Other relevant information:

Manage to solve it by changing the lines containing "yaml.FullLoader" to SafeLoader in config.py and service/jargon/loader.py.

@DarkFenX
Copy link
Member

DarkFenX commented Mar 25, 2019

From release notes:

Pyfa has updated python packages. If you're running pyfa from source, please make sure you update your environments to versions specified in requirements.txt

pip3 install pyyaml==5.1

Newer version is needed due to vulnerabilities in older version.

@blitzmann blitzmann added the invalid Issues that are opened but aren't really issues are normally marked as invalid (usually user error) label Mar 25, 2019
@blitzmann
Copy link
Collaborator

Closing as this is addressed in release notes and is environmental. @Roesjka if you're still having issues, feel free to continue to comment in this thread. Thanks!

@DeeDeeRanged
Copy link
Author

yaml/pyyaml#257
YAML has always had a safe_load method that can load a subset of YAML
without the risk of code execution. CVE-2017-18342 seems to suggest that load
should call safe_load by default. This is not feasible, because it will break
code that is using PyYAML as a full serialization language, not just for simple
config.

AFAICS yaml.SafeLoader is a safe bet. Main issue for Linux distros is that yaml 5.1 is not evenmentioned on the pyyaml.org site as that goes upto version 3.13 see issue
yaml/pyyaml#277.

Sofar I have not run into any issues by using yaml.SafeLoader, if I do I will report here asap.

@blitzmann
Copy link
Collaborator

blitzmann commented Mar 25, 2019

I'm unsure how the website being outdated has any bearing on being able to update to 5+ through pip? Unless you're using distro package manager to install these dependancies and they base it off the website?

@blitzmann blitzmann reopened this Mar 25, 2019
@DarkFenX
Copy link
Member

The website indeed seems to be barely maintained. Fyi, 5.1b3 is what I've installed during development. Looks like 5.1 has been released 12 days ago:

https://github.com/yaml/pyyaml/releases/tag/5.1
https://pypi.org/project/PyYAML/

I think that most likely version bump was a knee-jerk reaction to get rid of github security warnings. I also wasn't happy to install some dependencies via pip and tried to make simple solution to make it compatible with old pyyaml. Unfortunately, they changed API so it wasn't possible without doing version-specific logic.

Maybe we weren't using anything besides safe_load, but now as 5.1 is released i see little to no reason to switch it back.

@DarkFenX
Copy link
Member

Actually if safe loader is enough for our needs and it's available as yaml.SafeLoader in 3.x, we might switch to it indeed. Reopening to investigate it.

@DarkFenX DarkFenX reopened this Mar 25, 2019
@DarkFenX DarkFenX removed the invalid Issues that are opened but aren't really issues are normally marked as invalid (usually user error) label Mar 25, 2019
@DarkFenX
Copy link
Member

Fixed in 1a5fc31

@DarkFenX DarkFenX added bug Confirmed to be a bug fixed This issue has been fixed! Oh joy! labels Mar 26, 2019
@DarkFenX
Copy link
Member

Also marked it as a bug (even though it isn't imo).

@perlpunk
Copy link

also see yaml/pyyaml#265

@DarkFenX
Copy link
Member

DarkFenX commented Mar 26, 2019

@perlpunk thanks! Yes, I've already handled all API changes earlier when first installed 5.x beta. Within this issue, we just added compatibility with older pyyaml versions.

The other breakages do not affect us at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed to be a bug fixed This issue has been fixed! Oh joy!
Projects
None yet
Development

No branches or pull requests

4 participants