Skip to content

tests: match tcpdump RSTs to the data connection port pair - #22305

Closed
GrahamCampbell wants to merge 1 commit into
curl:masterfrom
GrahamCampbell:tcpdump-rst-port-pairs
Closed

tests: match tcpdump RSTs to the data connection port pair#22305
GrahamCampbell wants to merge 1 commit into
curl:masterfrom
GrahamCampbell:tcpdump-rst-port-pairs

Conversation

@GrahamCampbell

Copy link
Copy Markdown
Contributor

I noticed this issue while working on #22304. The vsftpd RST checks matched any captured RST involving one port number on either side, but tcpdump sees all of loopback while other pytest workers run — so an unrelated RST on a recycled ephemeral port can fail them (example). The trace line already names both endpoints, so return the pair from get_data_ports() and have get_rsts() match only RSTs between exactly those two ports.

@github-actions github-actions Bot added the tests label Jul 12, 2026
@bagder
bagder requested a review from icing July 12, 2026 17:59
@GrahamCampbell
GrahamCampbell force-pushed the tcpdump-rst-port-pairs branch 2 times, most recently from c559e8a to 7066f56 Compare July 13, 2026 12:18
@GrahamCampbell
GrahamCampbell marked this pull request as ready for review July 15, 2026 21:00
@GrahamCampbell
GrahamCampbell force-pushed the tcpdump-rst-port-pairs branch from 7066f56 to f4a48d9 Compare July 15, 2026 21:05
@GrahamCampbell
GrahamCampbell force-pushed the tcpdump-rst-port-pairs branch from f4a48d9 to 032e90e Compare July 21, 2026 21:54
Copilot AI review requested due to automatic review settings July 21, 2026 21:54
@GrahamCampbell

Copy link
Copy Markdown
Contributor Author

PR rebased. No changes made. Ready for review. :)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens the tcpdump-based FTPS/FTP shutdown tests to avoid flakiness from unrelated loopback TCP RSTs by matching RST packets only when they occur between the exact two endpoints (server/client port pair) of the FTP data connection.

Changes:

  • Update VsFTPD.get_data_ports() to return (remote_port, local_port) pairs parsed from curl’s “Established 2nd connection … from … port …” trace line.
  • Extend RunTcpDump.get_rsts() to support filtering by exact port pairs (port_pairs) in addition to the existing “either side matches” ports filtering.
  • Update vsftpd-related shutdown tests to pass port_pairs instead of ports.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/http/testenv/vsftpd.py Parse and return data-connection port pairs from curl trace lines.
tests/http/testenv/curl.py Add port_pairs filtering to tcpdump RST matching logic.
tests/http/test_30_vsftpd.py Use port_pairs for data-connection-specific RST assertions.
tests/http/test_31_vsftpds.py Use port_pairs for data-connection-specific RST assertions.
tests/http/test_32_ftps_vsftpd.py Use port_pairs for data-connection-specific RST assertions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jay
jay requested review from icing and removed request for icing July 24, 2026 06:41
@jay jay closed this in c886053 Jul 24, 2026
@jay

jay commented Jul 24, 2026

Copy link
Copy Markdown
Member

Thanks

@GrahamCampbell
GrahamCampbell deleted the tcpdump-rst-port-pairs branch July 24, 2026 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

4 participants