jleclanche / python-bna

Python implementation of the mobile Blizzard Authenticator (TOTP)
https://eu.battle.net/support/en/article/24520
MIT License
256 stars 38 forks source link

TypeError: option values must be strings #22

Open Dont-Copy-That-Floppy opened 5 years ago

Dont-Copy-That-Floppy commented 5 years ago

Line 136, self.config.set(serial, "secret", secret) fails on windows 10, python 3.7.3.

Dont-Copy-That-Floppy commented 5 years ago
Traceback (most recent call last):
  File "bna", line 281, in <module>
    main()
  File "C:\Users\Data\AppData\Local\Programs\Python\Python37-32\lib\site-packages\click\core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "C:\Users\Data\AppData\Local\Programs\Python\Python37-32\lib\site-packages\click\core.py", line 717, in main
    rv = self.invoke(ctx)
  File "C:\Users\Data\AppData\Local\Programs\Python\Python37-32\lib\site-packages\click\core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "C:\Users\Data\AppData\Local\Programs\Python\Python37-32\lib\site-packages\click\core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "C:\Users\Data\AppData\Local\Programs\Python\Python37-32\lib\site-packages\click\core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "C:\Users\Data\AppData\Local\Programs\Python\Python37-32\lib\site-packages\click\decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "bna", line 229, in restore
    ctx.obj.add_serial(serial, secret, set_default=set_default)
  File "bna", line 89, in add_serial
    self.set_secret(serial, secret)
  File "bna", line 136, in set_secret
    self.config.set(serial, "secret", secret)
  File "C:\Users\Data\AppData\Local\Programs\Python\Python37-32\lib\configparser.py", line 1197, in set
    self._validate_value_types(option=option, value=value)
  File "C:\Users\Data\AppData\Local\Programs\Python\Python37-32\lib\configparser.py", line 1182, in _validate_value_types
    raise TypeError("option values must be strings")
TypeError: option values must be strings
Dont-Copy-That-Floppy commented 5 years ago

Turns out the error isn't about the option values, but the value option. IE, (serial, "secret", secret), the last option is the value option, which is currently a byte array.

Dont-Copy-That-Floppy commented 5 years ago

Just fyi, your code is hard as shit to follow and debug. I have never seen python be written so hard to follow. Your restore function in http is where it's failing, but I can't debug it since it's not written for interpreted usage. No change I make in that function is reflected in the output.

jleclanche commented 5 years ago

I've no idea what you mean "not written for interpreted usage". If you want to debug it it's pretty easy to insert breakpoints or even just some print statements. I'll forgive your attitude since it sounds like you're very new at Python, but there are nicer ways to say what you just said.

Anyway, this works for me on Linux, so must be a Windows issue. If you can figure out what's happening I'll push a fix, but I don't have a Windows box to repro the issue.

Dont-Copy-That-Floppy commented 5 years ago

I mean when I change anything in any of the code inside the directory python-bna, I get zero output change. And when I say your code is hard as shit read, I mean it. The way you programmed this is way far different than standard programming procedure for python. I'm actually pretty fluent in python. I don't claim to be the best, but I've written some big stuff in python. A lot of the syntax you use in logical progression isn't python standard. You're probably a programmer from another language, and use the same logical flow, maybe some async web flow.

And I was nice, don't be so judgy, and self-righteous. I said nothing about you, I talked about your code. It's called feedback. "I'll forgive your attitude", when people don't ask for your approval, don't give it. It just makes you look like a super arrogant a-hole. No one adult cares what other random strangers think about them. If you want to code well, you should take my critic with stride. That's how you get better. Unless of course you think you're the best already. "it sounds like you're very new at Python", which you might already do. How a person responds to a perceived insult shows 100x more about that person than the person who gave the insult.

It might be one of the libs your using. Perhaps it's a different implementation on windows. Which of the libs you're using makes an OS call?

jleclanche commented 5 years ago

Alright I don't have time for this shit on a lib I only ever commit to for others. If you have a fix and want to point it out or PR it I'll take a look, but you're on your own debugging it.

Dont-Copy-That-Floppy commented 5 years ago

lol, github has thumbs down like facebook.

I'll spend some time on it this weekend. It's some call being made in the restore function inside http, and if it's working on linux, and not windows, then it must be an OS call being made by a 3rd party lib.

jleclanche commented 5 years ago

shrugs I don't use reactions, but don't be too surprised if other people find your posts abrasive.

then it must be an OS call being made by a 3rd party lib

My best guess is some windows input is sent to the lib as bytes instead of a string as on linux. But it's also possible your setup is broken, because I've not had this issue raised before (and your issues editing the files to change the input could be a symptom. tip, use pip install -e . to install a lib in editable mode…)

jleclanche commented 5 years ago

Sorry I'm wrong, #20 was opened the other day.

jleclanche commented 5 years ago

Could be a click issue.

s3bish commented 5 years ago

I guess this is a 'click' issue or an issue with how bna uses click.

This issue still exists. Tested on MAC and Linux, issue opener already tested on Win10.

Since this affects all OS and the last comment is from April I do not guess there will be fix coming? Did you open an issue at click? If yes, what are link and reactions? Did you try to fix the issue by changing the usage of click already? Maybe you can get help with it, this tool is pretty cool!

-- Some more thoughts:

The README is missing the installation description and some of the usage. Users do not only have to clone the repo, they have to install pip3, python-pip, click and bna. The recover parameter is not listed in the README, only the help option.

Please ping me after you fixed that issue with click. I would be glad to be allowed to advance your README in a way every one is able to use your great tool. The click issue has to fixed first though.

Thanks for your great tool and work!

Edit 1: In the other issue post (closed, this is the current one) you (jleclanche) said 'If you want to run the version from git, clone it locally and do pip install -e . in the bna directory'. Was does this do and why is it needed is not said.

Edit 2: There actually is a guide on Battlenet telling the missing information how to get it working: https://us.battle.net/forums/en/bnet/topic/20755608040?page=1

I was able to do it successfully, but even this guide is not complete. jleclanche you should definitely not develop inside your master branch. Please still feel free to ping me to enhance you documentation after you released the tested and working version 5. Until now, all users have to clone the 4.1.

jleclanche commented 5 years ago

@s3bish you can repro on linux? How? I've not been able to do that at all. If you can repro on linux, on current master please let me know how.