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

#8109: Server-side support for TLS Server Name Indication extension

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 1 month ago by jcea
Modified:
6 years, 6 months ago
Reviewers:
daniel.black, pitrou
CC:
jcea, AntoinePitrou, christian.heimes, grooverdan, devnull_psf.upfronthosting.co.za, piotr.dobrogost, dan, kyoshiddha_gmail.com, mpb
Visibility:
Public.

Patch Set 1 #

Total comments: 1

Patch Set 2 #

Total comments: 34

Patch Set 3 #

Total comments: 32

Patch Set 4 #

Patch Set 5 #

Patch Set 6 #

Total comments: 6

Patch Set 7 #

Patch Set 8 #

Patch Set 9 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/ssl.rst View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
Lib/test/test_ssl.py View 1 2 3 4 5 6 7 8 2 chunks +10 lines, -1 line 0 comments Download
Modules/_ssl.c View 1 2 3 4 5 6 7 8 1 chunk +18 lines, -12 lines 0 comments Download

Messages

Total messages: 7
dan
needs a CA cert with CA:True in basic constraints for the ssl negiotatation to work. ...
7 years, 1 month ago #1
AntoinePitrou
Sorry for the delay. Here are comments about the patch. Thank you! http://bugs.python.org/review/8109/diff/5989/Doc/library/ssl.rst File Doc/library/ssl.rst ...
6 years, 10 months ago #2
dan
thanks for the review. Fixed patch coming next. http://bugs.python.org/review/8109/diff/5989/Doc/library/ssl.rst File Doc/library/ssl.rst (right): http://bugs.python.org/review/8109/diff/5989/Doc/library/ssl.rst#newcode678 Doc/library/ssl.rst:678: .. ...
6 years, 10 months ago #3
AntoinePitrou
Here are a couple more comments. I think this is getting good. http://bugs.python.org/review/8109/diff/6793/Doc/library/ssl.rst File Doc/library/ssl.rst ...
6 years, 10 months ago #4
dan
All fixed referring to comments of the review. I looked through the cyclic reference docs. ...
6 years, 10 months ago #5
dan
Thanks for the weakref bits Antonine. http://bugs.python.org/review/8109/diff/6989/Modules/_ssl.c File Modules/_ssl.c (right): http://bugs.python.org/review/8109/diff/6989/Modules/_ssl.c#newcode1787 Modules/_ssl.c:1787: Py_VISIT(self->set_hostname); Isn't a ...
6 years, 9 months ago #6
AntoinePitrou
6 years, 9 months ago #7
Thanks for the comments. I will fix those mistakes.

http://bugs.python.org/review/8109/diff/6989/Modules/_ssl.c
File Modules/_ssl.c (right):

http://bugs.python.org/review/8109/diff/6989/Modules/_ssl.c#newcode1787
Modules/_ssl.c:1787: Py_VISIT(self->set_hostname);
On 2013/01/04 22:58:42, dan wrote:
> Isn't a #ifndef OPENSSL_NO_TLSEXT needed around Py_VISIT?

Yes, it is.

http://bugs.python.org/review/8109/diff/6989/Modules/_ssl.c#newcode2384
Modules/_ssl.c:2384: int ret = SSL_TLSEXT_ERR_OK;
On 2013/01/04 22:58:42, dan wrote:
> Since line 2444 was added there's no need to initialise this.

You're right, indeed.

http://bugs.python.org/review/8109/diff/6989/Modules/_ssl.c#newcode2453
Modules/_ssl.c:2453: Py_DECREF(ssl_socket);
On 2013/01/04 22:58:42, dan wrote:
> I'm not sure why you removed PyGILState_Release(gstate) here. Its not called
> before any of the goto error; statements.

Ouch. Copy/paste error, I guess.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+