fix: KIP-368 use receiver goroutine to process all sasl v1 responses #2234
Conversation
fixes defect in kip-368 implementation where existing incoming wire traffic would get confused with re-authentication traffic leading to unexpected disconnects and OOM errors.
|
@dnwe do you think is it possible to review and get this fix merged soon? The issue makes the re-auth feature not usable. |
dnwe
changed the title
dnwe
changed the title
|
@ppatierno no problem, I actually pushed it out as Version 1.34.0 because I wanted to include the KIP-345 (static membership) support to allow people to test that too |
|
@dnwe thank you very much! |
This is a fix for this SASL bug IBM/sarama#2234
This is a fix for this SASL bug IBM/sarama#2234
| b.connErr = b.authenticateViaSASLv1() | ||
| if b.connErr != nil { | ||
| close(b.responses) | ||
| err = b.conn.Close() | ||
| if err == nil { | ||
| DebugLogger.Printf("Closed connection to broker %s\n", b.addr) | ||
| } else { | ||
| Logger.Printf("Error while closing connection to broker %s: %s\n", b.addr, err) | ||
| } |
There was a problem hiding this comment.
It is quite a while ago so I don't have any useful memory. I think it was probably oversight. Looking at the code now, I see no reason why logging connErr would be a bad idea.
If I were making a change, I would double check that connErr never contains a secret.
My project focus has changed, so I'm not contributing to Sarama right now.
Fixes defect (#2233) in kip-368 implementation where existing incoming wire traffic would get confused with re-authentication traffic leading to unexpected disconnects and OOM errors.
The approach taken uses the existing receiver routine to process all SASL v1+ responses for both authentication and re-authentication. For SASL v0, the existing behaviour is maintained.