Skip to content
This repository has been archived by the owner on Aug 11, 2020. It is now read-only.

quic: client should work when receives invalid preferred address #155

Closed
wants to merge 1 commit into from

Conversation

oyyd
Copy link
Contributor

@oyyd oyyd commented Oct 7, 2019

If we set preferredAddressPolicy to accept on QuicClientSession
but receives an invalid preferred address, e.g. we don't define
preferredAddress on the server, the QuicClientSession should still
work. Before this PR, the included test will cause segmetation fault.

the example in ngtcp2 always return 0 on select_preferred_address : https://github.com/ngtcp2/ngtcp2/blob/52d56a90a7766c71c68362dc85f5fdc866760f47/examples/client.cc#L673

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

If we set `preferredAddressPolicy` to `accept` on `QuicClientSession`
but receives an invalid preferred address, e.g. we don't define
`preferredAddress` on the server, the `QuicClientSession` should still
work. Before this PR, the included test will cause segmetation fault.
@@ -200,7 +200,7 @@ class SocketAddress {
}

char host[NI_MAXHOST];
if (uv_inet_ntop(af, binaddr, host, sizeof(host)) == 0)
if (uv_inet_ntop(af, binaddr, host, sizeof(host)) != 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to be missed from the last rebasing. Refs: https://github.com/nodejs/quic/pull/139/files

Copy link
Member

Choose a reason for hiding this comment

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

how odd... thanks for catching

jasnell pushed a commit that referenced this pull request Oct 9, 2019
If we set `preferredAddressPolicy` to `accept` on `QuicClientSession`
but receives an invalid preferred address, e.g. we don't define
`preferredAddress` on the server, the `QuicClientSession` should still
work. Before this PR, the included test will cause segmetation fault.

PR-URL: #155
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Oct 9, 2019

Landed

@jasnell jasnell closed this Oct 9, 2019
addaleax pushed a commit that referenced this pull request Dec 11, 2019
If we set `preferredAddressPolicy` to `accept` on `QuicClientSession`
but receives an invalid preferred address, e.g. we don't define
`preferredAddress` on the server, the `QuicClientSession` should still
work. Before this PR, the included test will cause segmetation fault.

PR-URL: #155
Reviewed-By: James M Snell <[email protected]>
juanarbol pushed a commit to juanarbol/quic that referenced this pull request Dec 17, 2019
If we set `preferredAddressPolicy` to `accept` on `QuicClientSession`
but receives an invalid preferred address, e.g. we don't define
`preferredAddress` on the server, the `QuicClientSession` should still
work. Before this PR, the included test will cause segmetation fault.

PR-URL: nodejs#155
Reviewed-By: James M Snell <[email protected]>
juanarbol pushed a commit to juanarbol/quic that referenced this pull request Dec 17, 2019
If we set `preferredAddressPolicy` to `accept` on `QuicClientSession`
but receives an invalid preferred address, e.g. we don't define
`preferredAddress` on the server, the `QuicClientSession` should still
work. Before this PR, the included test will cause segmetation fault.

PR-URL: nodejs#155
Reviewed-By: James M Snell <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants