apache / cordova-plugin-file-transfer

Apache Cordova File Transfer Plugin
https://cordova.apache.org/
Apache License 2.0
596 stars 887 forks source link

CB-10974 Cordova file transfer Content-Length header problem #141

Closed daserge closed 8 years ago

daserge commented 8 years ago

Fixed iOS logic to not to include Content-Length when chunkedMode=true Fixed Android logic preventing chunkedMode to be enabled for http uploads Added tests for chunkedMode Documented chunkedMode not supported on Windows Documented Uploading of an empty file is not supported for chunkedMode=true and multipart=false for iOS, pended the corresponding test

Jira issue

daserge commented 8 years ago

This PR requires https://github.com/apache/cordova-labs/pull/13

nikhilkh commented 8 years ago

LGTM.

royipr commented 7 years ago

Hi.

I think I found a bug regarding the issue.. When trying to upload a file with chuckedMode: false it seems to attach a Transfer-Encoding header by default. This causes the upload to aws s3 to fail: Message>A header you provided implies functionality that is not implemented</Message><Header>Transfer-Encoding</Header>

Take a look a this issue http://stackoverflow.com/questions/39780922/cordova-file-transfer-to-s3-aws-returns-501-error

daserge commented 7 years ago

@royipr - does it attach Transfer-Encoding for chunkedMode: false? It should do the opposite according to the code. Does switching to chunkedMode: true fix the issue?

royipr commented 7 years ago

@daserge According to the response it does attach Transfer-Encoding header. Is there a way to print the headers? I didn't saw them on adb logcat.

Maybe the problem is here? : https://github.com/apache/cordova-plugin-file-transfer/commit/9347606dd33fe07ea36799b4dd28804019c68835#diff-2a8a5fef3397df87ab538f028a5c6b50R1506

Maybe we should expect that the header will be undefined?

Switching to chunkMode: true didn't resolve the issue. I was able to solve it by downgrade to version 1.5.1

daserge commented 7 years ago

I've tested the file-transfer tests via logging out the headers from whatheaders.com (in filetransfer.spec.27) and from local server headers echo endpoint (in chunkedMode handling) - and Transfer-Encoding is not being included in the requests. The only exception - is using chunkedMode=true, which leads to Transfer-Encoding=chunked.

What Android OS version has the issue? I've tested this on 5.0.1 device.

royipr commented 7 years ago

I have tested that on 6.0.1, Sounds weird what you saying... I thought it might be related to AWS but if version 1.5.1 worked, there must be something broken with 1.6. If you need me to test more things let me know.

daserge commented 7 years ago

Oh, I think it might switch to chunkedMode=true because of HTTPS: https://github.com/apache/cordova-plugin-file-transfer/blob/2b6b6d9333848beb05a6128242701261467b5d69/src/android/FileTransfer.java#L432,L433

It would be great if you could try to use this PR + chunkedMode=false setting and let me know if it works for you: https://github.com/apache/cordova-plugin-file-transfer/pull/161

royipr commented 7 years ago

cool Ill check that later today, I think that it will solve the problem

craig-at-rsg commented 7 years ago

This fix creates a problem for WSGI applications - before this fix, they would happily accept chunked transfers, but after they return status 411 and complain about a missing Content-Length header.

raykin commented 7 years ago

same here.

E/FileTransfer(22411): {"code":1,"source":"file:\/\/\/data\/data\/com.deltux.kasa\/cache\/tmp_20161113_082947-1226504239.jpg","target":"https:\/\/s3.amazon
aws.com\/assets-kasa-dev\/","body":"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>MissingContentLength<\/Code><Message>You must provide the Cont
ent-Length HTTP header.<\/Message><RequestId>934F1CD36D065625<\/RequestId><HostId>ma9wYyhr3H0aplxC5KQCLQMopLQz9IzfXk8o7ooq0DWe9dZM8kbqeDNmOWyAGpMmP3W8DuMym
A4=<\/HostId><\/Error>","http_status":411,"exception":"https:\/\/s3.amazonaws.com\/assets-kasa-dev\/"}
E/FileTransfer(22411): java.io.FileNotFoundException: https://s3.amazonaws.com/assets-kasa-dev/
E/FileTransfer(22411):  at com.android.okhttp.internal.http.HttpURLConnectionImpl.getInputStream(HttpURLConnectionImpl.java:210)
E/FileTransfer(22411):  at com.android.okhttp.internal.http.DelegatingHttpsURLConnection.getInputStream(DelegatingHttpsURLConnection.java:210)
E/FileTransfer(22411):  at com.android.okhttp.internal.http.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:25)
E/FileTransfer(22411):  at org.apache.cordova.filetransfer.FileTransfer.getInputStream(FileTransfer.java:594)
E/FileTransfer(22411):  at org.apache.cordova.filetransfer.FileTransfer.access$500(FileTransfer.java:69)
E/FileTransfer(22411):  at org.apache.cordova.filetransfer.FileTransfer$1.run(FileTransfer.java:513)
E/FileTransfer(22411):  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
E/FileTransfer(22411):  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
E/FileTransfer(22411):  at java.lang.Thread.run(Thread.java:818)

another weird stuff is

adb logcat FileTransfer:* *:S

doesn't show debug log

btw, the Content-Length works on ios.

@craig-at-rsg do you find workaround on android?

vukdavid commented 7 years ago

Facing same HTTP 411 issue: "A request with the POST method requires a valid Content-Length header."

Using android with latest plugin versions: cordova-plugin-file-transfer@1.6.0 cordova-plugin-file@4.3.0

If I rollback to following versions it works fine: cordova-plugin-file-transfer@1.5.1 cordova-plugin-file@4.2.0

craig-at-rsg commented 7 years ago

@raykin Haven't found a workaround - reverted to the older version of this plugin.

cooperjbrandon commented 7 years ago

I have this same exact issue trying to upload to s3. I also got the error message :

You must provide the Content-Length HTTP header.

TLDR: options.chunkedMode = false; fixes it.

Longer: Not exactly sure, but maybe s3 doesn't like the Transfer-Encoding header? When doing conn.setFixedLengthStreamingMode(fixedLength);, I no longer get a 411. This is accomplished by setting chunkedMode to false. By doing that, this line runs: https://github.com/apache/cordova-plugin-file-transfer/blob/master/src/android/FileTransfer.java#L442 .

Another possibility is that s3 does allow chunked mode, but we need to set additional headers on the request to make it work.

daserge commented 7 years ago

@cooperjbrandon what is your plugin version?

cooperjbrandon commented 7 years ago

@daserge 1.6.2

cozzbie commented 7 years ago

Issue seems to be mostly an android problem because same code works fine on iOS. Using @cooperjbrandon advice and adding options.chunkedMode = false; to just the android code fixed the issue. Thanks.

v. 1.6.1