Skip to content

Commit

Permalink
Code clean-up of FileTransfer
Browse files Browse the repository at this point in the history
- Fix warnings about toLowerCase()
- Don't assume connections are HTTP (fails for file://)
- Use StringBuilder
- Remove no-ops of disconnect() & keep-alive
  • Loading branch information
agrieve committed Feb 12, 2013
1 parent db099e7 commit 8ab7278
Showing 1 changed file with 45 additions and 44 deletions.
89 changes: 45 additions & 44 deletions framework/src/org/apache/cordova/FileTransfer.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ Licensed to the Apache Software Foundation (ASF) under one

import java.io.ByteArrayOutputStream;
import java.io.Closeable;
import java.io.DataOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
Expand All @@ -33,6 +32,7 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLConnection;
import java.net.URLDecoder;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
Expand Down Expand Up @@ -200,7 +200,7 @@ private void upload(final String source, final String target, JSONArray args, Ca
callbackContext.sendPluginResult(new PluginResult(PluginResult.Status.IO_EXCEPTION, error));
return;
}
final boolean useHttps = url.getProtocol().toLowerCase().equals("https");
final boolean useHttps = url.getProtocol().equals("https");

final RequestContext context = new RequestContext(source, target, callbackContext);
synchronized (activeRequests) {
Expand Down Expand Up @@ -258,7 +258,6 @@ public void run() {

// Use a post method.
conn.setRequestMethod("POST");
conn.setRequestProperty("Connection", "Keep-Alive");
conn.setRequestProperty("Content-Type", "multipart/form-data;boundary=" + BOUNDARY);

// Set the cookies on the response
Expand Down Expand Up @@ -291,37 +290,35 @@ public void run() {
* Store the non-file portions of the multipart data as a string, so that we can add it
* to the contentSize, since it is part of the body of the HTTP request.
*/
String extraParams = "";
StringBuilder beforeData = new StringBuilder();
try {
for (Iterator<?> iter = params.keys(); iter.hasNext();) {
Object key = iter.next();
if(!String.valueOf(key).equals("headers"))
{
extraParams += LINE_START + BOUNDARY + LINE_END;
extraParams += "Content-Disposition: form-data; name=\"" + key.toString() + "\";";
extraParams += LINE_END + LINE_END;
extraParams += params.getString(key.toString());
extraParams += LINE_END;
beforeData.append(LINE_START).append(BOUNDARY).append(LINE_END);
beforeData.append("Content-Disposition: form-data; name=\"").append(key.toString()).append('"');
beforeData.append(LINE_END).append(LINE_END);
beforeData.append(params.getString(key.toString()));
beforeData.append(LINE_END);
}
}
} catch (JSONException e) {
Log.e(LOG_TAG, e.getMessage(), e);
}

extraParams += LINE_START + BOUNDARY + LINE_END;
extraParams += "Content-Disposition: form-data; name=\"" + fileKey + "\";" + " filename=\"";
byte[] extraBytes = extraParams.getBytes("UTF-8");

String midParams = "\"" + LINE_END + "Content-Type: " + mimeType + LINE_END + LINE_END;
String tailParams = LINE_END + LINE_START + BOUNDARY + LINE_START + LINE_END;
byte[] fileNameBytes = fileName.getBytes("UTF-8");
beforeData.append(LINE_START).append(BOUNDARY).append(LINE_END);
beforeData.append("Content-Disposition: form-data; name=\"").append(fileKey).append("\";");
beforeData.append(" filename=\"").append(fileName).append('"').append(LINE_END);
beforeData.append("Content-Type: ").append(mimeType).append(LINE_END).append(LINE_END);
byte[] beforeDataBytes = beforeData.toString().getBytes("UTF-8");
byte[] tailParamsBytes = (LINE_END + LINE_START + BOUNDARY + LINE_START + LINE_END).getBytes("UTF-8");


// Get a input stream of the file on the phone
InputStream sourceInputStream = getPathFromUri(source);

int stringLength = extraBytes.length + midParams.length() + tailParams.length() + fileNameBytes.length;
Log.d(LOG_TAG, "String Length: " + stringLength);
int stringLength = beforeDataBytes.length + tailParamsBytes.length;
if (sourceInputStream instanceof FileInputStream) {
fixedLength = (int) ((FileInputStream)sourceInputStream).getChannel().size() + stringLength;
progress.setLengthComputable(true);
Expand All @@ -333,7 +330,7 @@ public void run() {
// It also causes OOM if HTTPS is used, even on newer devices.
boolean useChunkedMode = chunkedMode && (Build.VERSION.SDK_INT < Build.VERSION_CODES.FROYO || useHttps);
useChunkedMode = useChunkedMode || (fixedLength == -1);

if (useChunkedMode) {
conn.setChunkedStreamingMode(MAX_BUFFER_SIZE);
// Although setChunkedStreamingMode sets this header, setting it explicitly here works
Expand All @@ -343,19 +340,20 @@ public void run() {
conn.setFixedLengthStreamingMode(fixedLength);
}

DataOutputStream dos = null;
conn.connect();

OutputStream sendStream = null;
try {
dos = new DataOutputStream( conn.getOutputStream() );
sendStream = conn.getOutputStream();
synchronized (context) {
if (context.aborted) {
return;
}
context.currentOutputStream = dos;
context.currentOutputStream = sendStream;
}
//We don't want to change encoding, we just want this to write for all Unicode.
dos.write(extraBytes);
dos.write(fileNameBytes);
dos.writeBytes(midParams);
sendStream.write(beforeDataBytes);
totalBytes += beforeDataBytes.length;

// create a buffer of maximum size
int bytesAvailable = sourceInputStream.available();
Expand All @@ -367,9 +365,9 @@ public void run() {

long prevBytesRead = 0;
while (bytesRead > 0) {
totalBytes += bytesRead;
result.setBytesSent(totalBytes);
dos.write(buffer, 0, bytesRead);
sendStream.write(buffer, 0, bytesRead);
totalBytes += bytesRead;
if (totalBytes > prevBytesRead + 102400) {
prevBytesRead = totalBytes;
Log.d(LOG_TAG, "Uploaded " + totalBytes + " of " + fixedLength + " bytes");
Expand All @@ -386,17 +384,21 @@ public void run() {
}

// send multipart form data necessary after file data...
dos.writeBytes(tailParams);
dos.flush();
sendStream.write(tailParamsBytes);
totalBytes += tailParamsBytes.length;
sendStream.flush();
} finally {
safeClose(sourceInputStream);
safeClose(dos);
safeClose(sendStream);
}
context.currentOutputStream = null;
Log.d(LOG_TAG, "Sent " + totalBytes + " of " + fixedLength);

//------------------ read the SERVER RESPONSE
String responseString;
int responseCode = conn.getResponseCode();
Log.d(LOG_TAG, "response code: " + responseCode);
Log.d(LOG_TAG, "response headers: " + conn.getHeaderFields());
InputStream inStream = null;
try {
inStream = getInputStream(conn);
Expand All @@ -407,8 +409,7 @@ public void run() {
context.currentInputStream = inStream;
}


ByteArrayOutputStream out = new ByteArrayOutputStream();
ByteArrayOutputStream out = new ByteArrayOutputStream(Math.max(1024, conn.getContentLength()));
byte[] buffer = new byte[1024];
int bytesRead = 0;
// write bytes to file
Expand Down Expand Up @@ -459,8 +460,6 @@ public void run() {
https.setHostnameVerifier(oldHostnameVerifier);
https.setSSLSocketFactory(oldSocketFactory);
}

conn.disconnect();
}
}
}
Expand All @@ -476,7 +475,7 @@ private static void safeClose(Closeable stream) {
}
}

private static InputStream getInputStream(HttpURLConnection conn) throws IOException {
private static InputStream getInputStream(URLConnection conn) throws IOException {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.HONEYCOMB) {
return new DoneHandlerInputStream(conn.getInputStream());
}
Expand Down Expand Up @@ -527,13 +526,15 @@ private static SSLSocketFactory trustAllHosts(HttpsURLConnection connection) {
return oldFactory;
}

private static JSONObject createFileTransferError(int errorCode, String source, String target, HttpURLConnection connection) {
private static JSONObject createFileTransferError(int errorCode, String source, String target, URLConnection connection) {

Integer httpStatus = null;
int httpStatus = 0;

if (connection != null) {
try {
httpStatus = connection.getResponseCode();
if (connection instanceof HttpURLConnection) {
httpStatus = ((HttpURLConnection)connection).getResponseCode();
}
} catch (IOException e) {
Log.w(LOG_TAG, "Error getting HTTP status code from connection.", e);
}
Expand Down Expand Up @@ -602,7 +603,7 @@ private void download(final String source, final String target, JSONArray args,
callbackContext.sendPluginResult(new PluginResult(PluginResult.Status.IO_EXCEPTION, error));
return;
}
final boolean useHttps = url.getProtocol().toLowerCase().equals("https");
final boolean useHttps = url.getProtocol().equals("https");

if (!Config.isUrlWhiteListed(source)) {
Log.w(LOG_TAG, "Source URL is not in white list: '" + source + "'");
Expand All @@ -622,7 +623,7 @@ public void run() {
if (context.aborted) {
return;
}
HttpURLConnection connection = null;
URLConnection connection = null;
HostnameVerifier oldHostnameVerifier = null;
SSLSocketFactory oldSocketFactory = null;

Expand Down Expand Up @@ -654,10 +655,12 @@ public void run() {
}
// Return a standard HTTP connection
else {
connection = (HttpURLConnection) url.openConnection();
connection = url.openConnection();
}

connection.setRequestMethod("GET");
if (connection instanceof HttpURLConnection) {
((HttpURLConnection)connection).setRequestMethod("GET");
}

//Add cookie support
String cookie = CookieManager.getInstance().getCookie(source);
Expand Down Expand Up @@ -743,8 +746,6 @@ public void run() {
https.setHostnameVerifier(oldHostnameVerifier);
https.setSSLSocketFactory(oldSocketFactory);
}

connection.disconnect();
}
}
}
Expand Down

0 comments on commit 8ab7278

Please sign in to comment.