search

saguziel / Kryptose

A password manager
2 stars 0 forks source link

SSL socket exception makes the client crash #23

Closed AMarcedone closed 9 years ago

AMarcedone commented 9 years ago

From the requirements document

Sending packets to the server that are not well formed according to our communication protocol (or not valid TLS packets) will cause the thread dealing with that connection to terminate (IT ACTUALLY CRASHES, but i was trying to be diplomatic). This does not leave the server in an inconsistent state, as requests are handled atomically, and the exception gets correctly logged.

Not sure wether we should handle it more gracefully or not. @jnshi

jnshi commented 9 years ago

How does it crash? In ClientHandler.listen, it catches all the malformed input exceptions, logs it, and returns null. When ClientHandler.run sees a null Request, it terminates.

I should fine-tune the uncaught exception handler though. And also think about how fault-tolerant/atomic the data store writes are. On Mar 19, 2015 10:04 PM, "AMarcedone" notifications@github.com wrote:

From the requirements document

Sending packets to the server that are not well formed according to our communication protocol (or not valid TLS packets) will cause the thread dealing with that connection to terminate (IT ACTUALLY CRASHES, but i was trying to be diplomatic). This does not leave the server in an inconsistent state, as requests are handled atomically, and the exception gets correctly logged.

Not sure wether we should handle it more gracefully or not. @jnshi https://github.com/jnshi

— Reply to this email directly or view it on GitHub https://github.com/saguziel/Kryptose/issues/23.

AMarcedone commented 9 years ago

That's what Alex said. I didn't test it. Just send an httprequest to the server for the browser to test it On Mar 19, 2015 10:18 PM, "jnshi" notifications@github.com wrote:

How does it crash? In ClientHandler.listen, it catches all the malformed input exceptions, logs it, and returns null. When ClientHandler.run sees a null Request, it terminates.

I should fine-tune the uncaught exception handler though. And also think about how fault-tolerant/atomic the data store writes are. On Mar 19, 2015 10:04 PM, "AMarcedone" notifications@github.com wrote:

From the requirements document

Sending packets to the server that are not well formed according to our communication protocol (or not valid TLS packets) will cause the thread dealing with that connection to terminate (IT ACTUALLY CRASHES, but i was trying to be diplomatic). This does not leave the server in an inconsistent state, as requests are handled atomically, and the exception gets correctly logged.

Not sure wether we should handle it more gracefully or not. @jnshi https://github.com/jnshi

— Reply to this email directly or view it on GitHub https://github.com/saguziel/Kryptose/issues/23.

— Reply to this email directly or view it on GitHub https://github.com/saguziel/Kryptose/issues/23#issuecomment-83856175.

jnshi commented 9 years ago

Yeah, I don't know what behavior is more elegant than logging the error and terminating the thread.

AMarcedone commented 9 years ago

Ok. That's fine. I must have probably misunderstood things. I tested it myself and sounds fine

On Thu, Mar 19, 2015 at 10:31 PM, jnshi notifications@github.com wrote:

Yeah, I don't know what behavior is more elegant than logging the error and terminating the thread.

— Reply to this email directly or view it on GitHub https://github.com/saguziel/Kryptose/issues/23#issuecomment-83858995.