Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
heyrex opened this issue Jul 19, 2023 · 13 comments · Fixed by #7306
Closed

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

heyrex opened this issue Jul 19, 2023 · 13 comments · Fixed by #7306
Labels
needs reproduction needs reproducible example to illustrate the issue

Comments

@heyrex
Copy link

heyrex commented Jul 19, 2023

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:

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

@ionitron-bot ionitron-bot bot added the triage label Jul 19, 2023
@5uper
Copy link
Contributor

5uper commented Jul 19, 2023

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

@heyrex
Copy link
Author

heyrex commented Jul 19, 2023

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

@jcesarmobile jcesarmobile added the needs reproduction needs reproducible example to illustrate the issue label Jul 19, 2023
@ionitron-bot
Copy link

ionitron-bot bot commented Jul 19, 2023

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 ionitron-bot bot removed the triage label Jul 19, 2023
@Ionitron Ionitron added needs reply needs reply from the user and removed needs reply needs reply from the user labels Jul 19, 2023
@jcesarmobile jcesarmobile added the needs reply needs reply from the user label Jul 19, 2023
@heyrex
Copy link
Author

heyrex commented Jul 20, 2023

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

@Ionitron Ionitron removed the needs reply needs reply from the user label Jul 20, 2023
@jcesarmobile jcesarmobile added the needs reply needs reply from the user label Jul 24, 2023
@Ionitron Ionitron removed the needs reply needs reply from the user label Jul 26, 2023
@efuturetoday
Copy link

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 👍

@jankei
Copy link

jankei commented Aug 22, 2023

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

@GGAlanSmithee
Copy link

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?

@FrancescoPaiola
Copy link

Any update on this?

@dv-alan
Copy link

dv-alan commented Jan 17, 2024

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.

michaelwolz added a commit to michaelwolz/capacitor that referenced this issue Feb 12, 2024
Removes new line characters `\r\n` which are incorretly appended to the
data of multipart/form-data requests. This change removes these new line
characters in order to comply with the specifications, as defined here:
https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.2

Fixes: ionic-team#6748
michaelwolz added a commit to michaelwolz/capacitor that referenced this issue Feb 12, 2024
Removes new line characters `\r\n` which are incorrectly appended to the
data of multipart/form-data requests. This change removes these new line
characters in order to comply with the specifications, as defined here:
https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.2

Fixes: ionic-team#6748
@rezendeneto

This comment was marked as off-topic.

@fluxsaas
Copy link

fluxsaas commented Mar 5, 2024

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)!)
             }
 
         }
-- 

Copy link

ionitron-bot bot commented Apr 13, 2024

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 ionitron-bot bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs reproduction needs reproducible example to illustrate the issue
Projects
None yet