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

Fix port override in connect(). #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stephen-mw
Copy link

@stephen-mw stephen-mw commented May 29, 2019

At the moment, supplying a port to the connect() function is a no-op. It will always be replaced with the default port 5222 because pick_dns_answer always returns a default port.

It looks to me like the logic is reversed. If a port was explicitly supplied to connect then it should always be used. If not, fall back to whatever you get from the pick_dns_answer.

Feel free to close this an implement in another way if that's what you prefer.

How to repro:

xmpp = slixmpp.ClientXMPP(username, password)
xmpp.connect(address=("example.com", 9999))
xmpp.process()

DEBUG    Using selector: KqueueSelector
DEBUG    Loaded Plugin: RFC 6120: Stream Feature: STARTTLS
DEBUG    Loaded Plugin: RFC 6120: Stream Feature: Resource Binding
DEBUG    Loaded Plugin: RFC 3920: Stream Feature: Start Session
DEBUG    Loaded Plugin: RFC 6121: Stream Feature: Roster Versioning
DEBUG    Loaded Plugin: RFC 6121: Stream Feature: Subscription Pre-Approval
DEBUG    Loaded Plugin: RFC 6120: Stream Feature: SASL
DEBUG    Event triggered: connecting
DEBUG    DNS: Querying example.com for AAAA records.
DEBUG    DNS: Querying example.com for A records.
DEBUG    Connection failed: [Errno 61] Connect call failed ('2606:2800:220:1:248:1893:25c8:1946', 5222, 0, 0)
DEBUG    Event triggered: connection_failed

Note that the port fell back to the default 5222:

DEBUG    Connection failed: [Errno 61] Connect call failed ('2606:2800:220:1:248:1893:25c8:1946', 5222, 0, 0)

@xaimepardal
Copy link

I am waiting for this merge

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