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

decode encrypted root.App.main.context.dispatcher.stores #1253

Merged
merged 4 commits into from Dec 19, 2022

Conversation

Rogach
Copy link
Contributor

@Rogach Rogach commented Dec 18, 2022

WIP decryption code for issue #1246

However this doesn't fix the issue completely, since QuoteSummaryStore is not found in the decrypted data - probably the data is now stored in some other place.

@Rogach
Copy link
Contributor Author

Rogach commented Dec 18, 2022

I was using an incorrect ticker value - with correct ticker value scraping seems to work without problems (for my limited use-cases).

@Rogach
Copy link
Contributor Author

Rogach commented Dec 18, 2022

But there seems to be some small differences - for example, if previously yfinance.Ticker("^IXIC").info["regularMarketPrice"] returned the value directly, now it returns a dict {'raw': 10705.414, 'fmt': '10,705.41'}.

@havedill
Copy link

havedill commented Dec 18, 2022

Confirming that i loaded this into my environment with pip install 'yfinance @ git+https://github.com/ranaroussi/yfinance.git@refs/pull/1253/head'

It is working, but I agree I had to define ['raw'] in a decent number of places.

@jesusignazio
Copy link

pycryptodome is now a dependency as well.

@Rogach
Copy link
Contributor Author

Rogach commented Dec 18, 2022

pycryptodome is now a dependency as well

I'm not sure where to add it, so I need some help from the maintainers (maybe in setup.py? but what version boundary should I use?).

@Rogach
Copy link
Contributor Author

Rogach commented Dec 18, 2022

Ah, so the "raw" troubles were due to me deleting bunch of code that was actually needed. Sorry about that.

@ValueRaider
Copy link
Collaborator

I've just pushed a change to fix returned data structure, now the tests are passing. No worries @Rogach

Minimum version should be 3.6.6 or later because older contain a AES decryption bug. I'll update setup.py etc, then give others a chance to question.

@ValueRaider
Copy link
Collaborator

ValueRaider commented Dec 18, 2022

One more thing @Rogach . This also needs to be backported to branch release/0.1 (utils.py get_json()), because branch main is intended for the next big release. I don't mind doing this, I just want to give you chance in case you want to be in commit log. You will still get full credit in PRs and release notes.

fredrik-corneliusson added a commit to fredrik-corneliusson/yfinance-tz-cache that referenced this pull request Dec 18, 2022
@ValueRaider
Copy link
Collaborator

ValueRaider commented Dec 18, 2022

Regarding cryptography vs pycryptodome, is there any strong argument to choose one over other? cryptography has the high-level interface but this solution requires the low-level; it has larger community but pycryptodome has been free of known CVEs for longest time (unless pycryptodome has more unknown CVEs from less eyes).
@fredrik-corneliusson

EDIT: I have a Discussion pending in pycryptodome, hopefully they can answer because I can't find a decent answer anywhere.

@galashour
Copy link

Many thanks guys, this is much appreciated.

Is a new 'fixed' release is expected soon, or should we manually update a specific file on our local setup?

@ValueRaider
Copy link
Collaborator

Wait for a fix, it will come soon. We just need to decide with crypto module to use.

@fredrik-corneliusson
Copy link
Contributor

Regarding cryptography vs pycryptodome, is there any strong argument to choose one over other? cryptography has the high-level interface but this solution requires the low-level; it has larger community but pycryptodome has been free of known CVEs for longest time (unless pycryptodome has more unknown CVEs from less eyes). @fredrik-corneliusson

Sorry for causing confusion, I worked on this fix before I saw @Rogach already had PR.
I only made my version as I had issues installing the crypto lib I thought the code in the comment used (required compiling).
But now that @Rogach has a working PR and pycryptodome seems to install on windows without issues my branch can be scrapped if there is no other compelling reason to use cryptography lib.
I committed the code anyway in my repo but I was not planning to make a PR for it, forgot how good GitHub is at sniffing out commit messages in forks.

@ValueRaider
Copy link
Collaborator

I decided to put both in the code but defaulting to cryptography just so I can get a release out. Will tidy when I get a good answer.

@Rogach
Copy link
Contributor Author

Rogach commented Dec 19, 2022

@ValueRaider Sorry, was away from computer. Thanks for backporting the code!

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

6 participants