capacitor icon indicating copy to clipboard operation
capacitor copied to clipboard

bug: multipart/form-data encoding error in CapacitorUrlRequest.swift

Open heyrex opened this issue 1 year ago • 10 comments

The recently introduced FormData support in CapacitorHttp incorrectly encodes form values on iOS (works correctly for Android).

On iOS getRequestDataFromFormData() incorrectly adds \r\n below the value and omits it above. multipart/form-data encoding requires an empty line between the headers and the value. There should be no empty line after the value. It should be immediately proceeded by the boundary.

The two lines referenced here should be reversed: https://github.com/ionic-team/capacitor/blob/2e43323410bab7c3381a0f11a6fd3880bde03a70/ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift#L135C1-L136C56

It also looks like an additional \r\n is being appended to any data where type == "base64File". See: https://github.com/ionic-team/capacitor/blob/3cd7196ec10066a98561e5d1b1166d8c1133f13a/ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift#L131

heyrex avatar Jul 19 '23 06:07 heyrex

this is fixed in v5.2.x with PR https://github.com/ionic-team/capacitor/pull/6722

5uper avatar Jul 19 '23 06:07 5uper

this is fixed in v5.2.x with PR #6722

@5uper The pull request you reference only partially fixes the issue. That PR fixes the syntax of the multipart/form-data encoding but there is still data corruption because of the additional \r\n before the boundary.

This is the additional fix required: https://github.com/heyrex/capacitor-heyrex/pull/1/files

heyrex avatar Jul 19 '23 07:07 heyrex

Further reading: https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.2 https://stackoverflow.com/questions/4238809/example-of-multipart-form-data https://www.ietf.org/rfc/rfc2388.txt

heyrex avatar Jul 19 '23 07:07 heyrex

This issue needs more information before it can be addressed. In particular, the reporter needs to provide a minimal sample app that demonstrates the issue. If no sample app is provided within 15 days, the issue will be closed. Please see the Contributing Guide for how to create a Sample App. Thanks! Ionitron 💙

ionitron-bot[bot] avatar Jul 19 '23 11:07 ionitron-bot[bot]

No further time is available for this so we'll leave our fix available for anyone experiencing the same issue.

To any capacitor users experiencing this problem with Capacitor v4.8.1 or v5.2.x:

  1. Open ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift
  2. Find the getRequestDataFromFormData function.
  3. Modify as follows:
 if type == "base64File" {
    let fileName = item["fileName"]
    let fileContentType = item["contentType"]
    data.append("\r\n--\(boundary)\r\n".data(using: .utf8)!)
    data.append("Content-Disposition: form-data; name=\"\(key!)\"; filename=\"\(fileName!)\"\r\n".data(using: .utf8)!)
    data.append("Content-Type: \(fileContentType!)\r\n".data(using: .utf8)!)
    data.append("Content-Transfer-Encoding: binary\r\n".data(using: .utf8)!)
    data.append("\r\n".data(using: .utf8)!)
    data.append(Data(base64Encoded: value)!)
} else if type == "string" {
    data.append("\r\n--\(boundary)\r\n".data(using: .utf8)!)
    data.append("Content-Disposition: form-data; name=\"\(key!)\"\r\n".data(using: .utf8)!)
    data.append("\r\n".data(using: .utf8)!)
    data.append(value.data(using: .utf8)!)
}
  1. npx cap sync ios
  2. Open xcode and click "Product -> Clean Build Folder"
  3. Run your app.

Alternatively, modify the last line before "return" to remove the first "\r\n":

data.append("--\(boundary)--\r\n".data(using: .utf8)!)
return data

heyrex avatar Jul 20 '23 02:07 heyrex

No further time is available for this so we'll leave our fix available for anyone experiencing the same issue.

To any capacitor users experiencing this problem with Capacitor v4.8.1 or v5.2.x:

  1. Open ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift
  2. Find the getRequestDataFromFormData function.
  3. Modify as follows:
 if type == "base64File" {
    let fileName = item["fileName"]
    let fileContentType = item["contentType"]
    data.append("\r\n--\(boundary)\r\n".data(using: .utf8)!)
    data.append("Content-Disposition: form-data; name=\"\(key!)\"; filename=\"\(fileName!)\"\r\n".data(using: .utf8)!)
    data.append("Content-Type: \(fileContentType!)\r\n".data(using: .utf8)!)
    data.append("Content-Transfer-Encoding: binary\r\n".data(using: .utf8)!)
    data.append("\r\n".data(using: .utf8)!)
    data.append(Data(base64Encoded: value)!)
} else if type == "string" {
    data.append("\r\n--\(boundary)\r\n".data(using: .utf8)!)
    data.append("Content-Disposition: form-data; name=\"\(key!)\"\r\n".data(using: .utf8)!)
    data.append("\r\n".data(using: .utf8)!)
    data.append(value.data(using: .utf8)!)
}
  1. npx cap sync ios
  2. Open xcode and click "Product -> Clean Build Folder"
  3. Run your app.

Alternatively, modify the last line before "return" to remove the first "\r\n":

data.append("--\(boundary)--\r\n".data(using: .utf8)!)
return data

+1 for your work. We need this asap merged 👍

efuturetoday avatar Jul 26 '23 13:07 efuturetoday

A possible solution until this issue is resolved is to create a post-install script to remove that line

jankei avatar Aug 22 '23 23:08 jankei

In what sense does this issue "require more information before it can be addressed"? It's pretty clear what's going on, and super easy to replicate - post a form from an iOS device.

The suggested solution works (thanks @heyrex) but is modifying the package at build time really the intended way to use Capcitor?

GGAlanSmithee avatar Dec 05 '23 06:12 GGAlanSmithee

Any update on this?

FrancescoPaiola avatar Jan 17 '24 13:01 FrancescoPaiola

Any update on this? I noticed that in my case it's appending a \r\n to my values, resulting in a value containing formatting characters. I'm using version capacitor core/ios with verison 5.6.0.

dv-alan avatar Jan 17 '24 15:01 dv-alan

Maybe not 100% related to this, but I've noticed that there is an issue with handling special characters (e.g. ö, ä, ü) in multipart/form-data requests in android. Here's the PR that fixes the problem: https://github.com/ionic-team/capacitor/pull/7219

rezendeneto avatar Feb 26 '24 18:02 rezendeneto

if you want to use the tmp solution with a post_install hook, you can do it with:

post_install do |installer|
  # Apply path to /ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift
  patch_file_path = File.join(__dir__, 'boundary_patch.patch')
  if File.exist?(patch_file_path)
    `patch --directory ../../node_modules/@capacitor < ' #{patch_file_path}`
    puts "Applied patch to CapacitorUrlRequest.swift"
  else
    puts "Patch file not found: #{patch_file_path}"
  end
end

with the patch file boundary_patch.patch:

---
 ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift b/ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift
index 51331e3b..afd457e4 100644
--- a/ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift
+++ b/ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift
@@ -119,22 +119,17 @@ open class CapacitorUrlRequest: NSObject, URLSessionTaskDelegate {
             if type == "base64File" {
                 let fileName = item["fileName"]
                 let fileContentType = item["contentType"]
-
                 data.append("\r\n--\(boundary)\r\n".data(using: .utf8)!)
                 data.append("Content-Disposition: form-data; name=\"\(key!)\"; filename=\"\(fileName!)\"\r\n".data(using: .utf8)!)
                 data.append("Content-Type: \(fileContentType!)\r\n".data(using: .utf8)!)
                 data.append("Content-Transfer-Encoding: binary\r\n".data(using: .utf8)!)
                 data.append("\r\n".data(using: .utf8)!)
-
                 data.append(Data(base64Encoded: value)!)
-
-                data.append("\r\n".data(using: .utf8)!)
             } else if type == "string" {
                 data.append("\r\n--\(boundary)\r\n".data(using: .utf8)!)
                 data.append("Content-Disposition: form-data; name=\"\(key!)\"\r\n".data(using: .utf8)!)
                 data.append("\r\n".data(using: .utf8)!)
                 data.append(value.data(using: .utf8)!)
-                data.append("\r\n".data(using: .utf8)!)
             }
 
         }
-- 

fluxsaas avatar Mar 05 '24 19:03 fluxsaas

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.

ionitron-bot[bot] avatar Apr 13 '24 17:04 ionitron-bot[bot]