Detect SSL option errors at start up #547

Merged
merged 1 commit into from Jun 29, 2012

2 participants

@evanj

Feel free to reject this, as I can easily add these checks to my own code.

Previously, ssl_option errors aren't detected until a client connects. I've run into this more than once, where I screwed up some configuration or was missing a file, and thought the server was happy. This does some basic validation at startup, so the server crashes immediately on startup instead.

Ideally, we would use ssl.wrap_socket to actually try to read the key and certificate. However, this is a pain in the ass because it needs a connected socket. I decided that was too much work.

@bdarnell
tornadoweb member

This is a good change, but the test needs a little work. It should explicitly clean up its temporary file, and the docs say that NamedTemporaryFile cannot be opened a second time on windows (which is very confusing to me; I'm not sure why else you'd care that the file has a name). Or you could just avoid the temporary file and use the existing test.crt when you need a file that exists.

@evanj

Good point: Using a file that already exists is a lot better! I copied the code from tornado.testing to find test.crt relative to file. I amended my commit to attempt to keep cleaner history. Let me know if there are other changes I should make.

@bdarnell
tornadoweb member

Looks like your amended commit didn't come through - the version I see now is still using NamedTemporaryFile.

@evanj evanj TCPServer: Do some validation of ssl_options
Previously, errors aren't detected until a client connects.
73d26a4
@evanj

Oops. I screwed it up on my end; that's what I get for trying to be fancy. It should be there now. Thanks!

@bdarnell bdarnell merged commit a240c76 into tornadoweb:master Jun 29, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment