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

Fix IPv6-only bugs on SMTP settings #16155

Merged
merged 1 commit into from Aug 29, 2023
Merged

Fix IPv6-only bugs on SMTP settings #16155

merged 1 commit into from Aug 29, 2023

Conversation

evilham
Copy link
Contributor

@evilham evilham commented Aug 22, 2023

While there, do it in such a fashion that we both document and prepare the groundwork for similar issues relating to direct usage of reactor.connectTCP, which lead to IPv6 incompatibilities.

Closes #7720

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

@evilham evilham requested a review from a team as a code owner August 22, 2023 09:17
@evilham
Copy link
Contributor Author

evilham commented Aug 22, 2023 •
edited

This is a more manageable, realistic version of #11508, adding some ground work while also fixing a very specific bug.

I also noticed on the fork that the workflow fails, but it is currently also failing on the develop branch the same way.

clokep
clokep reviewed Aug 24, 2023
View reviewed changes
Copy link
Contributor

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks somewhat reasonable, but I have a few questions.

tests/handlers/test_send_email.py Show resolved Hide resolved
tests/handlers/test_send_email.py Outdated Show resolved Hide resolved
tests/unittest.py Outdated Show resolved Hide resolved
tests/unittest.py Outdated Show resolved Hide resolved
tests/unittest.py Outdated Show resolved Hide resolved
synapse/handlers/send_email.py Outdated Show resolved Hide resolved
@clokep clokep added the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Aug 25, 2023
@evilham evilham requested a review from clokep August 28, 2023 12:22
@evilham evilham force-pushed the smtp-ipv6 branch 2 times, most recently from 3ac4604 to e0c054c Compare August 28, 2023 13:40
@clokep clokep removed the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Aug 28, 2023
clokep
clokep reviewed Aug 28, 2023
View reviewed changes
tests/unittest.py Outdated Show resolved Hide resolved
@evilham evilham requested a review from clokep August 28, 2023 15:21
clokep
clokep reviewed Aug 28, 2023
View reviewed changes
Copy link
Contributor

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks good overall, just a couple of small nits.

tests/handlers/test_send_email.py Outdated Show resolved Hide resolved
synapse/handlers/send_email.py Outdated Show resolved Hide resolved
tests/server.py Outdated Show resolved Hide resolved
tests/server.py Outdated Show resolved Hide resolved
tests/server.py Outdated Show resolved Hide resolved
@evilham evilham requested a review from clokep August 29, 2023 08:24
@evilham evilham force-pushed the smtp-ipv6 branch 2 times, most recently from 2dc4757 to 8f59480 Compare August 29, 2023 08:45
clokep
clokep approved these changes Aug 29, 2023
View reviewed changes
Copy link
Contributor

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks good, minus one last comment!

synapse/handlers/send_email.py Outdated Show resolved Hide resolved
@evilham
Fix IPv6-only bugs on SMTP settings
c18fe46 
While there, do it in such a fashion that we both document and prepare
the groundwork for similar issues relating to direct usage of
reactor.connectTCP, which lead to IPv6 incompatibilities.

Closes matrix-org#7720

Signed-off-by: Nico Schottelius <foss@ungleich.ch>
@evilham
Copy link
Contributor Author

evilham commented Aug 29, 2023

Done, thanks a lot for the review :-)

@clokep clokep merged commit 63b51ef into matrix-org:develop Aug 29, 2023
37 checks passed
@clokep
Copy link
Contributor

clokep commented Aug 29, 2023

Thanks for handling this! 🎉

@glyph
Copy link
Contributor

glyph commented Aug 29, 2023

yaaay, super happy to see more HostnameEndpoint adoption!

hughns pushed a commit that referenced this pull request Sep 4, 2023
@evilham @hughns
Support IPv6-only SMTP servers (#16155)
7a9835a 
Use Twisted HostnameEndpoint to connect to SMTP servers (instead
of connectTCP/connectSSL) which properly supports IPv6-only servers.
DMRobertson pushed a commit that referenced this pull request Sep 5, 2023
Merge tag 'v1.92.0rc1' into develop
02bc590 
- Add configuration setting for CAS protocol version. Contributed by Aurélien Grimpard. ([\#15816](#15816))
- Suppress notifications from message edits per [MSC3958](matrix-org/matrix-spec-proposals#3958). ([\#16113](#16113))
- Return a `Retry-After` with `M_LIMIT_EXCEEDED` error responses. ([\#16136](#16136))
- Add `last_seen_ts` to the [admin users API](https://matrix-org.github.io/synapse/latest/admin_api/user_admin_api.html). ([\#16218](#16218))
- Improve resource usage when sending data to a large number of remote hosts that are marked as "down". ([\#16223](#16223))

- Fix IPv6-related bugs on SMTP settings, adding groundwork to fix similar issues. Contributed by @evilham and @telmich (ungleich.ch). ([\#16155](#16155))
- Fix a spec compliance issue where requests to the `/publicRooms` federation API would specify `include_all_networks` as a string. ([\#16185](#16185))
- Fix inaccurate error message while attempting to ban or unban a user with the same or higher PL by spliting the conditional statements. Contributed by @leviosacz. ([\#16205](#16205))
- Fix a rare bug that broke looping calls, which could lead to e.g. linearly increasing memory usage. Introduced in v1.90.0. ([\#16210](#16210))
- Fix a long-standing bug where uploading images would fail if we could not generate thumbnails for them. ([\#16211](#16211))
- Fix a long-standing bug where we did not correctly back off from servers that had "gone" if they returned 4xx series error codes. ([\#16221](#16221))

- Update links to the [matrix.org blog](https://matrix.org/blog/). ([\#16008](#16008))
- Document which [admin APIs](https://matrix-org.github.io/synapse/latest/usage/administration/admin_api/index.html) are disabled when experimental [MSC3861](matrix-org/matrix-spec-proposals#3861) support is enabled. ([\#16168](#16168))
- Document [`exclude_rooms_from_sync`](https://matrix-org.github.io/synapse/v1.92/usage/configuration/config_documentation.html#exclude_rooms_from_sync) configuration option. ([\#16178](#16178))

- Prepare unit tests for Python 3.12. ([\#16099](#16099))
- Fix nightly CI jobs. ([\#16121](#16121), [\#16213](#16213))
- Describe which rate limiter was hit in logs. ([\#16135](#16135))
- Simplify presence code when using workers. ([\#16170](#16170))
- Track per-device information in the presence code. ([\#16171](#16171), [\#16172](#16172))
- Stop using the `event_txn_id` table. ([\#16175](#16175))
- Use `AsyncMock` instead of custom code. ([\#16179](#16179), [\#16180](#16180))
- Improve error reporting of invalid data passed to `/_matrix/key/v2/query`. ([\#16183](#16183))
- Task scheduler: add replication notify for new task to launch ASAP. ([\#16184](#16184))
- Improve type hints. ([\#16186](#16186), [\#16188](#16188), [\#16201](#16201))
- Bump black version to 23.7.0. ([\#16187](#16187))
- Log the details of background update failures. ([\#16212](#16212))
- Cache device resync requests over replication. ([\#16241](#16241))

* Bump anyhow from 1.0.72 to 1.0.75. ([\#16141](#16141))
* Bump furo from 2023.7.26 to 2023.8.19. ([\#16238](#16238))
* Bump phonenumbers from 8.13.18 to 8.13.19. ([\#16237](#16237))
* Bump psycopg2 from 2.9.6 to 2.9.7. ([\#16196](#16196))
* Bump regex from 1.9.3 to 1.9.4. ([\#16195](#16195))
* Bump ruff from 0.0.277 to 0.0.286. ([\#16198](#16198))
* Bump sentry-sdk from 1.29.2 to 1.30.0. ([\#16236](#16236))
* Bump serde from 1.0.184 to 1.0.188. ([\#16194](#16194))
* Bump serde_json from 1.0.104 to 1.0.105. ([\#16140](#16140))
* Bump types-psycopg2 from 2.9.21.10 to 2.9.21.11. ([\#16200](#16200))
* Bump types-pyyaml from 6.0.12.10 to 6.0.12.11. ([\#16199](#16199))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@clokep clokep clokep approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Synapse can't connect to an IPv6-only mail server via hostname
3 participants
@evilham @clokep @glyph