Proper fix for rejecting ftp URL with /r/n.
This change superseedes previous (broken) attempt and is based
on openJdk change:
changeset: 17022:ada6fcb7cfd1
user: vtewari
date: Fri Feb 10 10:11:10 2017 +0530
summary: 8170222: Better transfers of files
Tests rewrite taken from tobiast@ fix in http://ag/2141666.
Altered to not check for the case with username and no password
(because in current code this results in NPE, not sure if expected,
will check further) and removed the case with only '\r' in it, which
is not handled by openJdk fix, but shouldn't cause issues.
Test: FtpURLConnectionTest
Bug: 35784677
Change-Id: Ifdf6ebca362d1a5836d32b6f9663eff6c0c8f6ac
Merged-In: I02c4b9194d395f901ef1e8e6154e06add25b51c9
(cherry picked from commit 382a4d3fa486b844f4fd42c16ee41d9a38ba52f0)
diff --git a/luni/src/test/java/libcore/java/net/FtpURLConnectionTest.java b/luni/src/test/java/libcore/java/net/FtpURLConnectionTest.java
index ff5d0b6..806ae73 100644
--- a/luni/src/test/java/libcore/java/net/FtpURLConnectionTest.java
+++ b/luni/src/test/java/libcore/java/net/FtpURLConnectionTest.java
@@ -19,8 +19,18 @@
import junit.framework.TestCase;
import java.io.IOException;
+import java.net.ServerSocket;
+import java.net.Socket;
import java.net.URL;
+import java.util.Arrays;
+import java.util.List;
import java.util.Locale;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+
+import sun.net.ftp.FtpLoginException;
/**
* Tests URLConnections for ftp:// URLs.
@@ -28,36 +38,48 @@
public class FtpURLConnectionTest extends TestCase {
private static final String FILE_PATH = "test/file/for/FtpURLConnectionTest.txt";
- private static final String USER = "user";
- private static final String PASSWORD = "password";
private static final String SERVER_HOSTNAME = "localhost";
- private static final String USER_HOME_DIR = "/home/user";
- // http://b/35784677
+ // http://b/35784677
public void testCRLFInUserinfo() throws Exception {
- int serverPort = 1234;
- // '/r/n' in the username, no password
- String url1String = String.format(Locale.US, "ftp://foo%%0D%%0Acommand@%s:%s/%s",
- SERVER_HOSTNAME, serverPort, FILE_PATH);
- // '/r/n' in the username with password
- String url2String = String.format(Locale.US, "ftp://foo%%0D%%0Acommand:foo@%s:%s/%s",
- SERVER_HOSTNAME, serverPort, FILE_PATH);
- // '/r/n' in the password
- String url3String = String.format(Locale.US, "ftp://foo:bar%%0D%%0Acommand@%s:%s/%s",
- SERVER_HOSTNAME, serverPort, FILE_PATH);
- // just '/r' in the password
- String url4String = String.format(Locale.US, "ftp://foo:bar%%0Dcommand@%s:%s/%s",
- SERVER_HOSTNAME, serverPort, FILE_PATH);
- // just '/n' in the username
- String url5String = String.format(Locale.US, "ftp://foo%%0Acommand:bar@%s:%s/%s",
- SERVER_HOSTNAME, serverPort, FILE_PATH);
+ List<String> encodedUserInfos = Arrays.asList(
+ // '\r\n' in the username with password
+ "user%0D%0Acommand:password",
+ // '\r\n' in the password
+ "user:password%0D%0Acommand",
+ // just '\n' in the password
+ "user:password%0Acommand",
+ // just '\n' in the username
+ "user%0Acommand:password"
+ );
+ for (String encodedUserInfo : encodedUserInfos) {
+ ExecutorService executor = Executors.newSingleThreadExecutor();
+ ServerSocket mockFtpServerSocket = new ServerSocket(0);
+ Future<Void> future = executor.submit(new Callable<Void>() {
+ @Override public Void call() throws Exception {
+ Socket clientSocket = mockFtpServerSocket.accept();
+ clientSocket.getOutputStream().write("220 o/".getBytes());
+ clientSocket.close();
+ return null;
+ }
+ });
+ executor.shutdown();
- for (String urlString : new String[]{ url1String, url2String, url3String, url4String,
- url5String }) {
+ String urlString = String.format(Locale.US, "ftp://%s@%s:%s/%s",
+ encodedUserInfo, SERVER_HOSTNAME, mockFtpServerSocket.getLocalPort(), FILE_PATH);
try {
- new URL(urlString).openConnection();
- fail();
- } catch(IOException expected) {}
+ new URL(urlString).openConnection().connect();
+ fail("Connection shouldn't have succeeded: " + urlString);
+ } catch (FtpLoginException expected) {
+ // The original message "Illegal carriage return" gets lost
+ // where FtpURLConnection.connect() translates the
+ // original FtpProtocolException into FtpLoginException.
+ assertEquals("Invalid username/password", expected.getMessage());
+ }
+
+ // Cleanup
+ future.get();
+ mockFtpServerSocket.close();
}
}
}
diff --git a/ojluni/src/main/java/sun/net/ftp/impl/FtpClient.java b/ojluni/src/main/java/sun/net/ftp/impl/FtpClient.java
index 61c4a95..fea9421 100755
--- a/ojluni/src/main/java/sun/net/ftp/impl/FtpClient.java
+++ b/ojluni/src/main/java/sun/net/ftp/impl/FtpClient.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -518,7 +518,8 @@
* @return <code>true</code> if the command was successful
* @throws IOException
*/
- private boolean issueCommand(String cmd) throws IOException {
+ private boolean issueCommand(String cmd) throws IOException,
+ sun.net.ftp.FtpProtocolException {
if (!isConnected()) {
throw new IllegalStateException("Not connected");
}
@@ -529,6 +530,12 @@
// ignore...
}
}
+ if (cmd.indexOf('\n') != -1) {
+ sun.net.ftp.FtpProtocolException ex
+ = new sun.net.ftp.FtpProtocolException("Illegal FTP command");
+ ex.initCause(new IllegalArgumentException("Illegal carriage return"));
+ throw ex;
+ }
sendServer(cmd + "\r\n");
return readReply();
}
@@ -1121,7 +1128,10 @@
*/
public void close() throws IOException {
if (isConnected()) {
- issueCommand("QUIT");
+ try {
+ issueCommand("QUIT");
+ } catch (FtpProtocolException e) {
+ }
loggedIn = false;
}
disconnect();
@@ -1899,7 +1909,8 @@
return null;
}
- private boolean sendSecurityData(byte[] buf) throws IOException {
+ private boolean sendSecurityData(byte[] buf) throws IOException,
+ sun.net.ftp.FtpProtocolException {
BASE64Encoder encoder = new BASE64Encoder();
String s = encoder.encode(buf);
return issueCommand("ADAT " + s);
