Opened 4 years ago

Closed 4 years ago

#17573 closed defect (fixed)

Update max_dl_per_request for IPv6 address length

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: easy, ipv6, TorCoreTeam201601, 201512-deferred
Cc: Actual Points:
Parent ID: #17811 Points:
Reviewer: Sponsor:

Description

max_dl_per_request depends on the maximum length of a textual IPv4 address.

To query IPv6 directories, the NS and MICRODESC calculations need to be updated for the maximum length of a textual IPv6 address in a URL. (That is, no brackets.)

While we're doing that, it would be nice to add a comment with the microdescriptor length calculation as well.

Child Tickets

Change History (11)

comment:1 Changed 4 years ago by teor

Component: - Select a componentTor

comment:2 Changed 4 years ago by nickm

I also question whether the squid limit actually exists or matters; it might turn out that we're just better off forgetting it.

comment:3 in reply to:  2 Changed 4 years ago by teor

Replying to nickm:

I also question whether the squid limit actually exists or matters; it might turn out that we're just better off forgetting it.

I wonder if increasing this would cause some relays (particularly those behind intercepting proxies) to be unable to download descriptors. Clients tunnel their connections, so it's not an issue for them.

All sorts of middleboxes have nasty ways of mangling URLs. Maybe we should decide a limit based on some kind of broader survey or standard, rather than squid.

For example, Apache's default is 8190, but some users report seeing around 4000 in other internet FAQs.
https://httpd.apache.org/docs/2.2/mod/core.html#limitrequestline

comment:4 Changed 4 years ago by teor

Keywords: TorCoreTeam201512 added

comment:5 Changed 4 years ago by teor

Keywords: ipv6 added
Parent ID: #17281#17811

comment:6 Changed 4 years ago by nickm

Keywords: TorCoreTeam201601 201512-deferred added; TorCoreTeam201512 removed

Perhaps in January?

comment:7 Changed 4 years ago by fergus

Status: newneeds_review

This turns out not to affect the limits very much.

See branch max_dl_per_request of https://github.com/fergus-dall/tor.git

comment:8 Changed 4 years ago by teor

Status: needs_reviewneeds_revision

Thanks for this patch.

Code review:

  • the original calculation was missing the port, which can be up to 6 characters extra: ":65535". Let's fix this existing bug while we're at it.
  • As far as I can tell, the IPv6 address should be in square brackets, to avoid ambiguity with an added ":port".

comment:9 in reply to:  8 Changed 4 years ago by fergus

Status: needs_revisionneeds_review

Replying to teor:

  • the original calculation was missing the port, which can be up to 6 characters extra: ":65535". Let's fix this existing bug while we're at it.
  • As far as I can tell, the IPv6 address should be in square brackets, to avoid ambiguity with an added ":port".

The actual request in directory_send_command won't include the square brackets because tor_dup_addr doesn't take a decorate parameter to pass to tor_addr_to_str. This is probably a bug.

I've pushed a commit fixing both of these.

comment:10 Changed 4 years ago by teor

Looks great, let's get it merged!

(Split off #18051 for the directory_send_command bug, which only matters once #17840 is applied, or perhaps for bridges on IPv6, or perhaps once IPv6-only relays exist.)

comment:11 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Okay, merged!

Note: See TracTickets for help on using tickets.