search

achillean / shodan-python

The official Python library for Shodan
https://developer.shodan.io
Other
2.5k stars 559 forks source link

stream: Remove set `decode_unicode` #164

Closed hyche closed 2 years ago

hyche commented 2 years ago

This doesn't handle well with every unicode strings, let only json.loads() do the decode instead.

hyche commented 2 years ago

The servers' responses are guaranteed to return UTF-8 unicode string. You're right, this works for Python version >= 3.6, and Python2. For Python version < 3.6, it will return this error TypeError: the JSON object must be str, not 'bytes'. However all Python version would return an error like ValueError: Unterminated string starting at: line 1 column 3961 (char 3960) for some banners, this is due to the decode_unicode=True transform the responses to string that json.loads() cannot parse.

So there are 2 ways to fix this:

@achillean what do you think ?

sebix commented 2 years ago

I'd also drop Python < 3.6 support

achillean commented 2 years ago

I'm nervous about dropping support for Python2 as we still get emails from people using Python2 but I'm ok if we limit it to Python >= 3.6 for Python3. I.e. I like option 2.

On Thu, Jan 6, 2022 at 3:01 PM Sebastian @.***> wrote:

I'd also drop Python < 3.6 support

— Reply to this email directly, view it on GitHub https://github.com/achillean/shodan-python/pull/164#issuecomment-1006931660, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC3I2BGKQOE6P6DFBKXWHLUUX7JVANCNFSM5LMQBYQA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

-- Shodan - http://www.shodanhq.com

Follow me on Twitter http://twitter.com/achillean

sebix commented 2 years ago

I'm nervous about dropping support for Python2 as we still get emails from people using Python2

Pooh. But: Limiting support to Python > 3.6 would not mean those people can't use the application any more, it just means, that they can't use the versions from now on, i.e. don't get the new features.

achillean commented 2 years ago

Yes, but it will create confusion for Python2 users when they think they're running on the latest version of the CLI yet aren't because of the installed Python version. If we don't need to break backwards-compatibility yet then we shouldn't. I would much rather do a 2.x release that's Python3 only with a clear break and new capabilities (ex. async support) than remove Python2 support for a minor unicode-related fix.

On Thu, Jan 6, 2022 at 3:28 PM Sebastian @.***> wrote:

I'm nervous about dropping support for Python2 as we still get emails from people using Python2

Pooh. But: Limiting support to Python > 3.6 would not mean those people can't use the application any more, it just means, that they can't use the versions from now on, i.e. don't get the new features.

— Reply to this email directly, view it on GitHub https://github.com/achillean/shodan-python/pull/164#issuecomment-1006947532, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC3I2AI6LJIMNCJ54HEDATUUYCPJANCNFSM5LMQBYQA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

-- Shodan - http://www.shodanhq.com

Follow me on Twitter http://twitter.com/achillean