[GitHub] cordova-plugin-file-transfer pull request: CB-11067 - Adds content...

2016-05-26 Thread daserge 
Github user daserge commented on the pull request:


https://github.com/apache/cordova-plugin-file-transfer/pull/140#issuecomment-221841464
  
@oddcb, yes, `Content-Length` should be added by 
`setFixedLengthStreamingMode`.
Which Android OS version had the issue?
Could you try it again with the master plugin version?
Do you use chunked mode for requests (Content-Length should not be present 
in case of chunked mode)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-file-transfer pull request: CB-11067 - Adds content...

2016-05-26 Thread oddcb 
Github user oddcb commented on the pull request:


https://github.com/apache/cordova-plugin-file-transfer/pull/140#issuecomment-221839381
  
Thanks for you reply @daserge 

So I'm not very into how file uploads are done properly, but the problem I 
fixed for our api here was adding the Content-Length into the code block 
starting at: 


https://github.com/apache/cordova-plugin-file-transfer/blob/9347606dd33fe07ea36799b4dd28804019c68835/src/android/FileTransfer.java#L406

Result:

```java
 

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);

if(filesize > -1)
beforeData.append("Content-Length: 
").append(filesize).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");
int stringLength = beforeDataBytes.length + 
tailParamsBytes.length;
```

As far as I can tell it is also what happens for iOS in 
https://github.com/apache/cordova-plugin-file-transfer/blob/9347606dd33fe07ea36799b4dd28804019c68835/src/ios/CDVFileTransfer.m#L222

``` [postBodyBeforeFile appendData:[[NSString 
stringWithFormat:@"Content-Length: %ld\r\n\r\n", (long)[fileData length]] 
dataUsingEncoding:NSUTF8StringEncoding]];```

History for us is just that our ios app was working with the file upload 
plugin as is, but when we added the android app with the plugin it did not send 
Content-Length as iOS did and hence our file upload API started to scream. We 
are using Jersey in our backend.

As far as I can tell your opinion is that this added code should not be 
required interacting with a standard api?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-file-transfer pull request: CB-11067 - Adds content...

2016-05-25 Thread daserge 
Github user daserge commented on the pull request:


https://github.com/apache/cordova-plugin-file-transfer/pull/140#issuecomment-221670591
  
@oddcb, thanks for the contribution!
This is not an issue though as the `Content-Length` header is set by 
[`setFixedLengthStreamingMode`](https://developer.android.com/reference/java/net/HttpURLConnection.html?hl=ru#setFixedLengthStreamingMode%28int%29)
 in 
https://github.com/apache/cordova-plugin-file-transfer/blob/9347606dd33fe07ea36799b4dd28804019c68835/src/android/FileTransfer.java#L438.
I've recently added 
[tests](https://github.com/apache/cordova-plugin-file-transfer/commit/9347606dd33fe07ea36799b4dd28804019c68835#diff-2a8a5fef3397df87ab538f028a5c6b50R1494)
 verifying `Content-Length`, `Content-Type` and `Transfer-Encoding` for 
multipart/non-multipart & chunked/non-chunked modes - and they are passing for 
Android and iOS.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-file-transfer pull request: CB-11067 - Adds content...

2016-05-17 Thread cordova-qa 
Github user cordova-qa commented on the pull request:


https://github.com/apache/cordova-plugin-file-transfer/pull/140#issuecomment-219928043
  
Cordova CI Build has one or more failures. 

**Commit** - 
[Link](https://github.com/apache/cordova-plugin-file-transfer/pull/140/commits/7673590ad65a673c7578f7eead78fd4bd083f05c)
**Dashboard** - 
[Link](http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5/)

| Builder Name  | Console Output | Test Report | Device Logs  |
| :---: | :---:  |   :---: | :---:|
| [Windows 8.1 Store]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-8.1-store/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-8.1-store/console)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-8.1-store/testReport/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-8.1-store/artifact/)
 |
| [Windows 10  Store]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-10-store/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-10-store/console)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-10-store/testReport/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-10-store/artifact/)
 |
| [Windows 8.1 Phone]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-8.1-phone/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-8.1-phone/console)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-8.1-phone/testReport/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=windows-slave,platformName=windows-8.1-phone/artifact/)
 |
| [iOS]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=mac-slave,platformName=ios/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=mac-slave,platformName=ios/console)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=mac-slave,platformName=ios/testReport/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=mac-slave,platformName=ios/artifact/)
 |
| [Android Mac]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=mac-slave,platformName=android/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=mac-slave,platformName=android/console)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=mac-slave,platformName=android/testReport/)
 | [Link]( 
http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-file-transfer-pr/5//label=mac-slave,platformName=android/artifact/)
 |
 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-file-transfer pull request: CB-11067 - Adds content...

2016-04-19 Thread nikhilkh 
Github user nikhilkh commented on the pull request:


https://github.com/apache/cordova-plugin-file-transfer/pull/140#issuecomment-212176107
  
@daserge  to help review. Looks like we might need some tests for this as 
well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-file-transfer pull request: CB-11067 - Adds content...

2016-04-13 Thread oddcb 
GitHub user oddcb opened a pull request:

https://github.com/apache/cordova-plugin-file-transfer/pull/140

CB-11067 - Adds content-length to multipart string if file length 
determined as also iOS plugin does

ICLA signed and sent per email.

https://issues.apache.org/jira/browse/CB-11067

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/oddcb/cordova-plugin-file-transfer 
CB-11067_add_file_content_length_to_multipart_if_present

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-plugin-file-transfer/pull/140.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #140


commit 7673590ad65a673c7578f7eead78fd4bd083f05c
Author: Odd Christer Brovig 
Date:   2016-04-13T08:10:16Z

CB-11067 - Adds content-length to multipart string if the file length is 
determined, as also the iOS plugin does.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org