Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(24)

Issue 2341: avoid ssl_eof error on SSL_WANT_READ and friends

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 4 months ago by Jacob
Modified:
3 months, 1 week ago
Reviewers:
bradfitz, djabberd
CC:
SVN Base:
http://code.sixapart.com/svn/djabberd/trunk/DJabberd
Visibility:
Public.

Description

Incrementing a counter on empty SSL reads is not the correct way to determine
ssl_eof
(http://code.sixapart.com/trac/djabberd/changeset/758)

Instead we should check the error code to see if it is one of the SSL errors for
which we are supposed to retry(
http://www.openssl.org/docs/ssl/SSL_get_error.html)

Patch Set 1

Total comments: 4

Patch Set 2 : fixes

Total comments: 1

Patch Set 3 : response to comments

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
lib/DJabberd/Connection.pm View 1 2 1 chunk 17 lines 0 comments Download
lib/DJabberd/Stanza/StartTLS.pm View 1 2 chunks 37 lines 0 comments Download

Messages

Total messages: 3
bradfitz
http://codereview.appspot.com/2341/diff/1/2 File lib/DJabberd/Stanza/StartTLS.pm (right): http://codereview.appspot.com/2341/diff/1/2#newcode74 Line 74: my $err = Net::SSLeay::get_error($ssl,-1); space after comma http://codereview.appspot.com/2341/diff/1/2#newcode85 ...
1 year, 4 months ago
Jacob
I've uploaded a new patch http://codereview.appspot.com/2341/diff/1/2 File lib/DJabberd/Stanza/StartTLS.pm (right): http://codereview.appspot.com/2341/diff/1/2#newcode74 Line 74: my $err = ...
1 year, 4 months ago
bradfitz
1 year, 4 months ago
http://codereview.appspot.com/2341/diff/61/82
File lib/DJabberd/Connection.pm (right):

http://codereview.appspot.com/2341/diff/61/82#newcode495
Line 495: # A call to 'should_we_close_on_zero_bytes' accounts for the
possibility
You didn't update this comment.

Otherwise this seems fine now, assuming you've tested it all.  See
http://danga.com/commit.bml and let's get you commit access.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld r483