Queue reads everywhere

[mtodd]

After #138 was merged, I realized that any reads from the socket are susceptible from the same problem as described in #136.

I've added a reproduction (as an integration test) to illustrate the point, and also includes a trivial fix based on replacing read usage with queued_read.

This needs to be done for any usage of read in Net::LDAP::Connection:

• setup_encryption
• bind_simple
• bind_sasl
• modify
• add
• rename
• delete

cc @jch @schaary

[jch]

Yup. Let me know when it's ready for review or if you'd like to take a few of the tasks.

[mtodd]

I was able to come up with a straightforward, if not simpler reproduction of the issue while introducing queued_read-specific unit tests. Still lacks full coverage but it'll be easier to do than the integration test path.

[mtodd]

FWIW, I'm starting to hate the queued_read method name and would like to rename it.

[jch]

Looks good to me. I didn't try running all of these, but just did a sanity check of the changes.

FWIW, I'm starting to hate the queued_read method name and would like to rename it.

Did you want to just change it back to read now that everything is using it? Something else?

[mtodd]

@jch read is an option, but that means renaming read to read_socket or something a little more low level, or going with read_message since its responsibility is to return a message matching the message ID.

[jch]

@mtodd your call. I'm fine with leaving it queued_read as is. It's an internal method that's only used within Connection, so it's low impact to change whenever we'd like. 🚢 when you're ready.

[mtodd]

@jch not going to block this PR on the method name, better to have a separate conversation about it.