release-request-556939d2-cc5e-453a-b797-8a7cb6dacac2-for-git_nyc-mr1-security-f-release-4118426 snap-temp-L72200000081371809
Change-Id: I5ffb5f07311cefd857dbb0c61112b10043b5be47
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);
diff --git a/ojluni/src/main/java/sun/net/www/protocol/ftp/FtpURLConnection.java b/ojluni/src/main/java/sun/net/www/protocol/ftp/FtpURLConnection.java
index fd96343..e2b7fa1 100755
--- a/ojluni/src/main/java/sun/net/www/protocol/ftp/FtpURLConnection.java
+++ b/ojluni/src/main/java/sun/net/www/protocol/ftp/FtpURLConnection.java
@@ -184,12 +184,6 @@
}
if (userInfo != null) { // get the user and password
- // Android-changed: Added a test for CR/LF presence in the userInfo
- if (userInfo.indexOf("\r") != -1 || userInfo.indexOf("\n") != -1) {
- throw new IOException("<CR> and/or <LF> characters in username and password are"
- + " not permitted");
- }
-
int delimiter = userInfo.indexOf(':');
if (delimiter == -1) {
user = ParseUtil.decode(userInfo);
