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 Yahoo data decryption #1297

Merged
merged 3 commits into from
Jan 14, 2023

Conversation

alexa-infra
Copy link
Contributor

Should fix #1291

@SamZhang02
Copy link

This seems to work on my local, thanks for the fix :)

@andrewkenreich
Copy link

My worry here is you are assuming that the key is just not the other two values in the dict - what if they add another key to that dict eventually? Could we check for the key value that is 128 char long?

Would still break when they change encryption off 128 but prevents if they added future things to the dict not related to this. (Also since you are already preventing if they change the name of this key)

For now though this works. This will probably be a cat and mouse game for awhile.

@ValueRaider ValueRaider changed the base branch from dev to main January 14, 2023 13:52
@ValueRaider ValueRaider changed the base branch from main to dev January 14, 2023 13:53
@ValueRaider
Copy link
Collaborator

ValueRaider commented Jan 14, 2023

Restored the "_cr" and "_cs" code path, in case data is coming from requests_cache. And clarified 'data' vs 'stores' naming, was confusing.

@ValueRaider
Copy link
Collaborator

@andrewkenreich I've incorporated your feedback, so yfinance should print a useful message if format changes again.

@ValueRaider
Copy link
Collaborator

Can I get confirmation that this works with my changes? Then will merge.

@dschaefer
Copy link

Worked for me.

@ValueRaider ValueRaider merged commit 47c579f into ranaroussi:dev Jan 14, 2023
@ValueRaider
Copy link
Collaborator

Great, merged into dev. Would be great if you can switch to dev and act as beta testers for me, as some other changes there.

dev = I believe is stable but needs stress-testing

@dschaefer
Copy link

Already done :). Looks good. Thanks!

@ValueRaider ValueRaider changed the title Fix stores decrypt Fix Yahoo data decryption Jan 14, 2023
@ValueRaider ValueRaider mentioned this pull request Jan 14, 2023
@james-stevens
Copy link

Well done, great work

If I was doing the code review (which I'm not, so ignore me as you please) I'd say a couple of things

  1. probably be neater to make decode_password a separate function (lines 52 to 75)
  2. seems to me if k not in ["context", "plugins"]] is vulnerable to breaking, so maybe a function that looks for appropriate looking hex strings might be more robust - but who knows what they will do in the future, so also maybe not

@no0be
Copy link

no0be commented Jan 16, 2023

Thanks everyone for the quick fix, just upgraded to the 0.2.4 package from pip3 and I confirm it works fine.

@cameroncochrane
Copy link

Thank you for fixing, much appreciated :)

@alexa-infra alexa-infra deleted the fix-stores-decryption branch January 19, 2023 20:34
@Sathiya-K
Copy link

This seems to work on my local, thanks for the fix :)

Can you please explain how did you fix it? I upgraded my yfinance to 0.1.93. Thanks in advance!

@Sathiya-K
Copy link

Thanks everyone for the quick fix, just upgraded to the 0.2.4 package from pip3 and I confirm it works fine.

Upgraded the yfinance library is it?

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.

10 participants