wking / rss2email

open-source tool for Windows, Mac OS and UNIX for getting news from RSS feeds in email
http://pypi.python.org/pypi/rss2email/
GNU General Public License v2.0
270 stars 48 forks source link

Force SSLv3 on smtplib.starttls() on Python 3.4 #30

Closed thiagoc closed 10 years ago

thiagoc commented 10 years ago

I was getting this error on Python 3.4:

Traceback (most recent call last):
  File "/usr/lib/python3.4/site-packages/rss2email/email.py", line 161, in smtp_send
    smtp.starttls()
  File "/usr/lib/python3.4/smtplib.py", line 688, in starttls
    server_hostname=server_hostname)
  File "/usr/lib/python3.4/ssl.py", line 364, in wrap_socket
    _context=self)
  File "/usr/lib/python3.4/ssl.py", line 578, in __init__
    self.do_handshake()
  File "/usr/lib/python3.4/ssl.py", line 805, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLError: [SSL: TLSV1_ALERT_DECODE_ERROR] tlsv1 alert decode error (_ssl.c:598)
wking commented 10 years ago

On Wed, Jun 11, 2014 at 08:48:01PM -0700, Thiago Coutinho wrote:

  • Force SSLv3 on smtplib.starttls() on Python 3.4

Thanks :). Two minor issues:

I also think it would be better to avoid re-connecting. It looks like starttls learned the context argument in 3.3 2. I'd rather have something like:

try: smtp.starttls(context=_ssl.SSLContext(…)) except TypeError: # Python 3.2 or earlier smtp.starttls()

I see no need to hardcode SSLv3. Perhaps a new (in rss2email.config):

('smtp-ssl-protocol', 'SSLv3')

which we'd use with:

protocol_name = config.get(section, 'smtp-ssl-protocol') protocol = getattr(ssl, 'PROTOCOL{}'.format(protocol_name)) try: smtp.starttls(context=_ssl.SSLContext(protocol=protocol)) except TypeError: # Python 3.2 or earlier smtp.starttls()

wking commented 10 years ago

On Fri, Jun 13, 2014 at 09:52:59AM -0700, Thiago Coutinho wrote:

Closed #30.

Closed? You can just force-push a revised branch if you want to update this pull request. Or did you close it for another reason?

thiagoc commented 10 years ago

Sorry @wking, I don't know how to force-push, can you guide me?

wking commented 10 years ago

On Fri, Jun 13, 2014 at 10:39:38AM -0700, Thiago Coutinho wrote:

I don't know how to force-push, can you guide me?

Sure. It looks like your pull request is from your master branch, so just make the fixup changes, stage them, and ‘git commit --amend’ to squash them onto your original commit. Then:

$ git push --force thiagoc master

(assuming ‘thiagoc’ is your remote name for https://github.com/thiagoc/rss2email). That says “please clobber whatever is currently on thiagoc/master to make it match my local master branch”.

wking commented 10 years ago

Waiting for a reroll ;).

thiagoc commented 10 years ago

And about the "Signed-off-by", I read that I have to use "git commit -s ...", it's correct?

wking commented 10 years ago

On Fri, Jun 13, 2014 at 10:56:15AM -0700, Thiago Coutinho wrote:

And about the "Signed-off-by", I read that I have to use "git commit -s ...", it's correct?

‘commit -s’ will add the Signed-off-by line to your commit message automatically, but you can just type it in by hand as well.

thiagoc commented 10 years ago

What do you mean by "ssl import should be alphebetized (so between smtplib and subprocess)"?

2014-06-13 15:05 GMT-03:00 W. Trevor King notifications@github.com:

On Fri, Jun 13, 2014 at 10:56:15AM -0700, Thiago Coutinho wrote:

And about the "Signed-off-by", I read that I have to use "git commit -s ...", it's correct?

‘commit -s’ will add the Signed-off-by line to your commit message automatically, but you can just type it in by hand as well.

— Reply to this email directly or view it on GitHub https://github.com/wking/rss2email/pull/30#issuecomment-46042104.

thiagoc

"O povo não deveria temer o governo. O governo é quem deveria temer o povo." V de Vingança

wking commented 10 years ago

On Fri, Jun 13, 2014 at 11:42:01AM -0700, Thiago Coutinho wrote:

What do you mean by "ssl import should be alphebetized (so between smtplib and subprocess)"?

The imports are sorted alphabetically, so it should be:

… import smtplib as _smtplib import ssl as _ssl import subprocess as _subprocess …

thiagoc commented 10 years ago

Let me know if there's something I'm missing.

wking commented 10 years ago

On Fri, Jun 13, 2014 at 12:22:27PM -0700, Thiago Coutinho wrote:

Let me know if there's something I'm missing.

Declaring smtp-ssl-protocol in rss2email.config 1. And I'd probably squash your two commits into a single commit, but I can do that if you don't want to bother with it.

thiagoc commented 10 years ago

And I'd probably squash your two commits into a single commit, but I can do that if you don't want to bother with it.

I can do that, but you have to explain how :)

wking commented 10 years ago

On Fri, Jun 13, 2014 at 12:54:05PM -0700, Thiago Coutinho wrote:

And I'd probably squash your two commits into a single commit, but I can do that if you don't want to bother with it.

I can do that, but you have to explain how :)

If you have ‘origin’ as the remote name for https://github.com/wking/rss2email, use

$ git reset origin/master

to say “leave my working directory alone, but forget about any local commits after origin/master”. Then stage your stuff from scratch, and commit as usual.

If you have comments you want to recycle from the old commit messages, you'll want to copy them somewhere safe (before resetting), so you have them to paste back into the new commit message when you write it (after resetting).

thiagoc commented 10 years ago

This is what I've done: I cloned your repo on Github and did a "git clone" on my repo, so "origin" points to my repo. I tried to do the following:

$ git remote add wking https://github.com/wking/rss2email.git

$ git reset wking/master
fatal: ambiguous argument 'wking/master': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

Now I'm stuck :)

2014-06-13 16:59 GMT-03:00 W. Trevor King notifications@github.com:

On Fri, Jun 13, 2014 at 12:54:05PM -0700, Thiago Coutinho wrote:

And I'd probably squash your two commits into a single commit, but I can do that if you don't want to bother with it.

I can do that, but you have to explain how :)

If you have ‘origin’ as the remote name for https://github.com/wking/rss2email, use

$ git reset origin/master

to say “leave my working directory alone, but forget about any local commits after origin/master”. Then stage your stuff from scratch, and commit as usual.

If you have comments you want to recycle from the old commit messages, you'll want to copy them somewhere safe (before resetting), so you have them to paste back into the new commit message when you write it (after resetting).

— Reply to this email directly or view it on GitHub https://github.com/wking/rss2email/pull/30#issuecomment-46053694.

thiagoc

"O povo não deveria temer o governo. O governo é quem deveria temer o povo." V de Vingança

wking commented 10 years ago

On Fri, Jun 13, 2014 at 01:20:26PM -0700, Thiago Coutinho wrote:

$ git remote add wking https://github.com/wking/rss2email.git

$ git reset wking/master fatal: ambiguous argument 'wking/master': unknown revision or path not in the working tree.

You're missing the ‘git fetch wking’ between the ‘remote’ and ‘reset’ calls.

thiagoc commented 10 years ago

haha sorry @wking I don't know what I just have done :smile:

Can you fix this? I'm confused.

wking commented 10 years ago

On Fri, Jun 13, 2014 at 02:47:39PM -0700, Thiago Coutinho wrote:

Can you fix this? I'm confused.

Here's what you have:

$ git log --graph --date-order --oneline --decorate

My guess is that you successfully created the squashed commit (2369d56), after which you should have force-pushed it to your public master. Instead, you merged the old master (7cdf6e2) and pushed the resulting merge (dc828cc). I'd suggest you recover your repositories with:

$ git checkout master $ git reset --hard 2369d56 $ git push -f thiagoc master

(assuming ‘thiagoc’ is the name for your https://github.com/thiagoc/rss2email remote).

In any case, I'm happy with 2369d56, which I'll float to the list for further feedback 1. If it's quiet for a week (which is likely), I'll merge it into master and cut a new release. Thanks :).

thiagoc commented 10 years ago
$ git checkout master
Already on 'master'
$ git reset --hard 2369d56 
HEAD is now at 2369d56 Added the option "smtp-ssl-protocol" to make STARTTLS work on Python 3.3+
$ git push -f origin master 
Total 0 (delta 0), reused 0 (delta 0)
To https://github.com/thiagoc/rss2email.git
 + dc828cc...2369d56 master -> master (forced update)

That's it?

wking commented 10 years ago

On Sat, Jun 14, 2014 at 03:56:13PM -0700, Thiago Coutinho wrote:

$ git checkout master Already on 'master' $ git reset --hard 2369d56 HEAD is now at 2369d56 Added the option "smtp-ssl-protocol" to make STARTTLS work on Python 3.3+ $ git push -f origin master Total 0 (delta 0), reused 0 (delta 0) To https://github.com/thiagoc/rss2email.git

  • dc828cc...2369d56 master -> master (forced update)

That's it?

That's it.

wking commented 10 years ago

Merged in be376a6f0607e9fa490d6c456eebf033405186ae (Merge branch 'ssl-protocol', 2014-06-30).