Opened 7 months ago

Last modified 6 months ago

#33919 needs_revision task

Write unit tests for channel_matches_target_addr_for_extend()

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.4.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: prop311, ipv6, technical-debt, outreachy-ipv6, tests
Cc: nickm Actual Points:
Parent ID: #33048 Points: 0.5
Reviewer: nickm Sponsor: Sponsor55-can


In #33817, we modify channel_matches_target_addr_for_extend() to take an IPv4 and an IPv6 address.

We also modify channel_get_for_extend() to take an IPv4 and an IPv6 address. It has existing tests, but they ignore IP addresses. (And mock the matches_target() method, which is called by channel_matches_target_addr_for_extend().)

It would be useful to have tests that check that a channel matches, if its IP address matches the IPv4 or IPv6 address. But it's not urgent. (And the actual changes to the function are pretty trivial.)

The setup for these tasks is a bit tricky, we need to set up a channel_tls_t and an associated connection_t with a real_addr. (Note that #33898 may change the name of real_addr.)

This task is not urgent or important. Anyone can do this task.

Child Tickets

Change History (3)

comment:1 Changed 6 months ago by MrSquanchee

Status: newneeds_review

comment:2 Changed 6 months ago by nickm

Reviewer: nickm

comment:3 Changed 6 months ago by nickm

Status: needs_reviewneeds_revision

I think this is mostly okay, except that this part makes me nervous.

  channel_tls_t *tlschan = tor_malloc_zero(sizeof(*tlschan));
  or_connection_t *orcon = tor_malloc_zero(sizeof(*orcon));

It isn't creating a real channel or a real connection, but instead is creating a partially initialized version of each. There's no actual guarantee in the rest of our code that this should work: it _happens_ to work, but that could change in the future.

Maybe new_fake_channel() and free_fake_channel() would be a good choice for setting up the channel-like object?

Note: See TracTickets for help on using tickets.