search

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-10771: Fixing failure when empty string passed as a value for opti… #133

Closed rakatyal closed 8 years ago

rakatyal commented 8 years ago

…on parameter in upload function

CreateUploadAsync fails when passed an empty string as a value for SetText function. It also fails when we skip specifying a value for it. So when a user passes an empty string, skip the parameter instead.

rakatyal commented 8 years ago

@riknoll, @jasongin: Please review.

riknoll commented 8 years ago

Is this the only workaround? If so, this quirk should definitely be documented.

rakatyal commented 8 years ago

I believe so. As mentioned, it fails on passing empty string as a value for SetText function. It also fails when we skip specifying a value for it. One way to avoid this is leaving out this chunk, but then we lose sending the key for the option parameter. I can document it as a quirk.

rakatyal commented 8 years ago

Updated to ignoring the parameter all together.

paulhickman-a365 commented 8 years ago

Won't using "params[key]" as the test make it also ignore non-string values such as 0 or boolean false? Testing for undefined, null and "" explicitly would be better.

rakatyal commented 8 years ago

Yes it would but the options parameter is supposed to have only string key-value pairs as mentioned in the README.

rakatyal commented 8 years ago

I tested it on Android and apparently we do accept non string values and I don't see any reason why we should not. So your suggestion sounds fair to me.

jasongin commented 8 years ago

The update to check for an empty toString() result (as discussed) looks good.

rakatyal commented 8 years ago

Merged here.