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

IPv6 support for endpoint.py #1690

Closed
wants to merge 1 commit into from
Closed

Conversation

glyph
Copy link
Contributor

@glyph glyph commented Dec 11, 2016

Similar to #1689, but for endpoint.py

Similar to matrix-org#1689, but for endpoint.py
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

4 similar comments
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@glyph glyph mentioned this pull request Dec 11, 2016
@@ -13,7 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from twisted.internet.endpoints import SSL4ClientEndpoint, TCP4ClientEndpoint
from twisted.internet.endpoints import HostnameEndpoint, wrapClientTLS
Copy link
Contributor

Choose a reason for hiding this comment

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

TCP4ClientEndpoint is used as a default value in in the SpiderEndpoint and SRVClientEndpoint init functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should probably be removed from there; TCP4ClientEndpoint is only interesting as a low-level implementation detail.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly pointing it out as another place where it should have been replaced with HostnameEndpoint, but since they're only instantiated in one place, I'm not sure whether it would be simpler to just remove the default value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha; yeah. I'd be inclined to eliminate the default, personally; I find that default args are useful a lot more rarely than they seem like they'd be useful :)

transport_endpoint = SSL4ClientEndpoint
endpoint_kw_args.update(sslContextFactory=ssl_context_factory)
def transport_endpoint(reactor, host, port):
return wrapClientTLS(ssl_context_factory, HostnameEndpoint(reactor, host, port))
Copy link
Contributor

Choose a reason for hiding this comment

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

This wrapper also needs to take a timeout parameter for use in HostnameEndpoint.

@kyrias
Copy link
Contributor

kyrias commented Dec 11, 2016

Looks good other than those small things, and I've tested it and it seems to be working!

@kyrias
Copy link
Contributor

kyrias commented Dec 11, 2016

Federation seems to be at least partially broken over IPv6 though. I can join rooms and send/accept invites over IPv6, but messages aren't being synced.

Edit: It seems like it's actually working after switching to a trusted SSL cert.

@kyrias kyrias mentioned this pull request Dec 12, 2016
kyrias added a commit to kyrias/synapse that referenced this pull request Dec 12, 2016
Signed-off-by: Johannes Löthberg <[email protected]>
@glyph
Copy link
Contributor Author

glyph commented Dec 13, 2016

Since this has been superseded, shall I close it?

@ara4n
Copy link
Member

ara4n commented Dec 18, 2016

@kyrias has consolidated this into #1696, and we'll try to land it from there. thanks @glyph!!

@ara4n ara4n closed this Dec 18, 2016
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.

4 participants