Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

net: use cached peername to resolve remote props #9366

Closed
wants to merge 1 commit into from

Conversation

jameshartig
Copy link

Allows socket.remote* properties to still be accessed even after the
socket is closed. Fixes #9287

@@ -53,12 +57,20 @@ server.listen(common.PORT, 'localhost', function() {
assert.equal(common.PORT, client.remotePort);
client.end();
});
client.on('close', function() {
assert.notEqual(-1, remoteAddrCandidates.indexOf(client2.remoteAddress));
Copy link

Choose a reason for hiding this comment

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

Should these two lines be checking client instead of client2?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch. Woops!

@cjihrig
Copy link

cjihrig commented Mar 10, 2015

One question about the test. I haven't run the code, but if the CI is happy, then this LGTM.

Allows socket.remote* properties to still be accessed even after the
socket is closed. Fixes nodejs#9287
@jameshartig
Copy link
Author

Good catch. Fixed the test and it still passes.

cjihrig pushed a commit that referenced this pull request Mar 16, 2015
Allows socket.remote* properties to still be accessed even after the
socket is closed.

Fixes: #9287
PR-URL: #9366
Reviewed-By: Colin Ihrig <[email protected]>
@cjihrig
Copy link

cjihrig commented Mar 16, 2015

Thanks! Landed in 8c38b07.

@cjihrig cjihrig closed this Mar 16, 2015
@tjfontaine tjfontaine reopened this Mar 16, 2015
cjihrig pushed a commit to nodejs/node that referenced this pull request Mar 16, 2015
Allows socket.remote* properties to still be accessed even after the
socket is closed.

Fixes: nodejs/node-v0.x-archive#9287
PR-URL: nodejs/node-v0.x-archive#9366
Reviewed-By: Colin Ihrig <[email protected]>
@jasnell
Copy link
Member

jasnell commented Aug 15, 2015

@cjihrig ... do you happen to know why this was reopened?

@cjihrig
Copy link

cjihrig commented Aug 15, 2015

I think that was a malfunction in the TJ Fontaine bot.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants