Skip to content

Fix handling of no remoteAddress available#4396

Merged
devinivy merged 3 commits intohapijs:masterfrom
joshkel:remote-addr-gone
Dec 1, 2022
Merged

Fix handling of no remoteAddress available#4396
devinivy merged 3 commits intohapijs:masterfrom
joshkel:remote-addr-gone

Conversation

@joshkel
Copy link
Copy Markdown
Contributor

@joshkel joshkel commented Nov 16, 2022

Fixes #4395

I'm unfamiliar with Hapi's test designs; please let me know if the test needs any changes.

Copy link
Copy Markdown
Contributor

@DJMcK DJMcK left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, @joshkel! Experiencing the same exception accessing request.info.remoteAddress which brought me here.

Not a core maintainer so take my suggestions with a pinch of salt :)


FWIW, I'm not entirely convinced that there is a need to go through the whole rigmarole of creating a connection and destroying it for this case.

We could just simplify things down to something along the lines of:

it('handles when client address is undefined', async () => {

    const server = Hapi.server();
    server.route({
        method: 'get',
        path: '/',
        handler(request) {

            delete request.raw.req.socket.remoteAddress;
            return request.info.remoteAddress === undefined;
        }
    });
    const { result } = await server.inject('/');
    expect(result).to.equal(true);
});

I believe delete should be safe to use in this scenario.

@hueniverse
Copy link
Copy Markdown
Contributor

@devinivy @kanongil This is causing productions issues for us as well. Anything we can do to help get this merged?

@devinivy
Copy link
Copy Markdown
Member

This looks pretty dang close— I should be able to land this within the next 24 hours.

@ssnigur
Copy link
Copy Markdown

ssnigur commented Nov 30, 2022

@devinivy any updates on when will this be merged? Thank you

Co-authored-by: David McKeown <[email protected]>
@devinivy devinivy added the bug Bug or defect label Dec 1, 2022
@devinivy devinivy added this to the 21.0.1 milestone Dec 1, 2022
@devinivy devinivy self-assigned this Dec 1, 2022
@devinivy devinivy merged commit a0efdc4 into hapijs:master Dec 1, 2022
@joshkel joshkel deleted the remote-addr-gone branch December 1, 2022 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug or defect

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error from remoteAddr if the socket goes away

5 participants