Skip to content

Conversation

@mina-asham
Copy link
Contributor

  • When attempting to read from a broken connection, Jedis could potentially try to read some corrupt data and interpret it according to the Redis protocol, this makes it throw an exception instead of trying that.
  • Jedis causes OutOfMemoryException after SocketTimeoutException #1747 describes that possible misinterpretations of the protocol can lead to trying to create very large arrays
  • Also refactor clientPause test, it was testing that clients fail, CLIENT PAUSE command pauses all clients, so they should be expected to be delayed rather than failed

- When attempting to read from a broken connection, Jedis could potentially try to read some corrupt data and interpret it according to the Redis protocol, this makes it throw an exception instead of trying that.
- redis#1747 describes that possible misinterpretations of the protocol can lead to trying to create very large arrays
- Also refactor clientPause test, it was testing that clients fail, CLIENT PAUSE command pauses all clients, so they should be expected to be delayed rather than failed
Copy link
Contributor

@sazzad16 sazzad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sazzad16 sazzad16 added this to the 3.1.0 milestone Jan 5, 2019
@sazzad16
Copy link
Contributor

sazzad16 commented Jan 5, 2019

@marcosnils @gkorland

@mina-asham
Copy link
Contributor Author

ping, it's quite a small diff...

@mina-asham
Copy link
Contributor Author

Another ping... @marcosnils @gkorland

@marcosnils marcosnils modified the milestones: 3.1.0, 3.0.2 Jan 21, 2019
@marcosnils
Copy link
Contributor

LGTM. @smadasu I've changed it to 3.0.2 as it's not adding anything new features.

@sazzad16
Copy link
Contributor

@marcosnils Sure!

BTW, It's @sazzad16, not @smadasu ;)

@suryavtkt
Copy link

what does it mean broken connection, what are the possibilities to broke a connection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants