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

IPv6 support #1696

Merged
merged 5 commits into from
Dec 19, 2016
Merged

IPv6 support #1696

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6e18805
IPv6 support for client.py
glyph Dec 11, 2016
9f07f4c
IPv6 support for endpoint.py
glyph Dec 11, 2016
d3bd948
Fixup for #1689 and #1690
kyrias Dec 12, 2016
0648e76
Remove spurious newline
kyrias Dec 12, 2016
a9c1b41
Bump twisted dependency
kyrias Dec 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 14 additions & 17 deletions synapse/http/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from canonicaljson import encode_canonical_json

from twisted.internet import defer, reactor, ssl, protocol, task
from twisted.internet.endpoints import SSL4ClientEndpoint, TCP4ClientEndpoint
from twisted.internet.endpoints import HostnameEndpoint, wrapClientTLS
from twisted.web.client import (
BrowserLikeRedirectAgent, ContentDecoderAgent, GzipDecoder, Agent,
readBody, PartialDownloadError,
Expand Down Expand Up @@ -386,26 +386,23 @@ def __init__(self, hs):

def endpointForURI(self, uri):
logger.info("Getting endpoint for %s", uri.toBytes())

if uri.scheme == "http":
return SpiderEndpoint(
reactor, uri.host, uri.port, self.blacklist, self.whitelist,
endpoint=TCP4ClientEndpoint,
endpoint_kw_args={
'timeout': 15
},
)
endpoint_factory = HostnameEndpoint
elif uri.scheme == "https":
tlsPolicy = self.policyForHTTPS.creatorForNetloc(uri.host, uri.port)
return SpiderEndpoint(
reactor, uri.host, uri.port, self.blacklist, self.whitelist,
endpoint=SSL4ClientEndpoint,
endpoint_kw_args={
'sslContextFactory': tlsPolicy,
'timeout': 15
},
)
tlsCreator = self.policyForHTTPS.creatorForNetloc(uri.host, uri.port)

def endpoint_factory(reactor, host, port, **kw):
return wrapClientTLS(
tlsCreator,
HostnameEndpoint(reactor, host, port, **kw))
else:
logger.warn("Can't get endpoint for unrecognised scheme %s", uri.scheme)
return None
return SpiderEndpoint(
reactor, uri.host, uri.port, self.blacklist, self.whitelist,
endpoint=endpoint_factory, endpoint_kw_args=dict(timeout=15),
Copy link
Member

Choose a reason for hiding this comment

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

what's the rationale for using dict() rather than {} here? i think the existing code style is to use {}...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just didn't change that part, but it seems dict() is used in a bunch of places as well

λ git grep ' dict\(' | wc -l
61

I can certainly change it to {} instead though.

Copy link
Member

Choose a reason for hiding this comment

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

looking through the results of that grep, none of those uses seem to be for constants like this, which is only why it jumped out as looking weird. it's trivial, but probably worth fixing. (disclaimer: i'm hardly a python expert, but trying to help fill in whilst @erikjohnston is out)

Copy link
Contributor

Choose a reason for hiding this comment

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

The rationale is as follows:

  1. This dictionary is going to be used as **kw, meaning it needs to be native str mapped to some object.
  2. To facilitate python 3 portability, all modules should slowly be getting as many __future__ imports as possible, to make the py2 environment and the py3 environment consistent. One such import is from __future__ import unicode_literals.
  3. If you rely on "" to produce a native str, this assumption will be broken with unicode_literals imported.
  4. However, dict(**kw) will produce kw unaltered; meaning, it will be a native str even if a quoted string would produce a unicode.

So this is generally a habit I believe one should get into for py3 hygiene.

Copy link
Member

Choose a reason for hiding this comment

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

okay, i'm happy to defer (and yield, for that matter) to python wisdom here ;)

Copy link

Choose a reason for hiding this comment

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

One such import is from __future__ import unicode_literals

FWIW, Guido says you shouldn't use unicode_literals and they're going to add a warning about this to the official docs. (See recent thread on python-dev.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hrm. I'm not sure I fully agree with that thread, but definitely unicode_literals is not without its pitfalls. Nevertheless, dict(**kw) is a clearer expression of the intent of "I intend to use this for keywords".

Copy link
Contributor

Choose a reason for hiding this comment

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

Thread here.

)


class SpiderHttpClient(SimpleHttpClient):
Expand Down
14 changes: 8 additions & 6 deletions synapse/http/endpoint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
from twisted.internet import defer
from twisted.internet.error import ConnectError
from twisted.names import client, dns
Expand Down Expand Up @@ -58,11 +58,13 @@ def matrix_federation_endpoint(reactor, destination, ssl_context_factory=None,
endpoint_kw_args.update(timeout=timeout)

if ssl_context_factory is None:
transport_endpoint = TCP4ClientEndpoint
transport_endpoint = HostnameEndpoint
default_port = 8008
else:
transport_endpoint = SSL4ClientEndpoint
endpoint_kw_args.update(sslContextFactory=ssl_context_factory)
def transport_endpoint(reactor, host, port, timeout):
return wrapClientTLS(
ssl_context_factory,
HostnameEndpoint(reactor, host, port, timeout=timeout))
default_port = 8448

if port is None:
Expand All @@ -80,7 +82,7 @@ class SpiderEndpoint(object):
Implements twisted.internet.interfaces.IStreamClientEndpoint.
"""
def __init__(self, reactor, host, port, blacklist, whitelist,
endpoint=TCP4ClientEndpoint, endpoint_kw_args={}):
endpoint=HostnameEndpoint, endpoint_kw_args={}):
self.reactor = reactor
self.host = host
self.port = port
Expand Down Expand Up @@ -118,7 +120,7 @@ class SRVClientEndpoint(object):
"""

def __init__(self, reactor, service, domain, protocol="tcp",
default_port=None, endpoint=TCP4ClientEndpoint,
default_port=None, endpoint=HostnameEndpoint,
endpoint_kw_args={}):
self.reactor = reactor
self.service_name = "_%s._%s.%s" % (service, protocol, domain)
Expand Down