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

Fix port override in connect().#19

Open
stephen-mw wants to merge 1 commit intopoezio:masterfrom
stephen-mw:master
Open

Fix port override in connect().#19
stephen-mw wants to merge 1 commit intopoezio:masterfrom
stephen-mw:master

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