Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(15)

Issue 11418035: FTP: Only issue EPSV command for non-IPv4 connections. (Closed)

Created:
6 years, 4 months ago by Paweł Hajdan Jr.
Modified:
6 years, 4 months ago
Reviewers:
wtc
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

FTP: Only issue EPSV command for non-IPv4 connections. For an IPv4 connection PASV is completely fine. Some servers become confused by EPSV: they send a success response, but our connection to data port times out, while with PASV it works fine. Note that it also might be an issue with firewall between us and the server. Whatever it is, EPSV confuses it, so we don't use it when it's not necessary. This also makes the unit test for FTP network transaction test both IPv4 and IPv6 flow. BUG=131847 TEST=Covered by net_unittests, also see bug. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=168427

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+202 lines, -104 lines) Patch
M net/ftp/ftp_network_transaction.cc View 1 chunk +9 lines, -0 lines 0 comments Download
M net/ftp/ftp_network_transaction_unittest.cc View 49 chunks +193 lines, -104 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Paweł Hajdan Jr.
6 years, 4 months ago (2012-11-16 17:46:47 UTC) #1
wtc
Patch set 1 LGTM. Thanks.
6 years, 4 months ago (2012-11-17 02:18:42 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/phajdan.jr@chromium.org/11418035/1
6 years, 4 months ago (2012-11-17 03:48:03 UTC) #3
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
6 years, 4 months ago (2012-11-17 12:25:19 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/phajdan.jr@chromium.org/11418035/1
6 years, 4 months ago (2012-11-17 13:28:14 UTC) #5
commit-bot: I haz the power
6 years, 4 months ago (2012-11-17 14:16:08 UTC) #6
Change committed as 168427

Powered by Google App Engine
This is Rietveld 408576698