fix(ios)!: solve FormData boundary issues #7306
Conversation
jcesarmobile
requested review from
andredestro,
markemer,
theproducer,
nflsilva,
ItsChaceD,
Steven0351 and
giralte-ionic
| if dataType == "file" { | ||
| guard let stringData = body as? String else { | ||
| throw CapacitorUrlRequestError.serializationError("[ data ] argument could not be parsed as string") | ||
| } | ||
| return Data(base64Encoded: stringData) | ||
| } else if dataType == "formData" { | ||
| return try getRequestDataFromFormData(body) | ||
| return try getRequestDataFromFormData(body, boundary!) |
There was a problem hiding this comment.
We sure this will always be non-nil?
There was a problem hiding this comment.
When using the patched fetch/XHR yeah, it will be there.
For direct API calls not so sure, but on Android there are no checks, so in any case it would crash on both platforms.
There was a problem hiding this comment.
Could easily turn this into:
} else if dataType == "formData", let boundary {to avoid the force unwrapping
There was a problem hiding this comment.
another solution could be to collapse the "multipart/form-data" check below into one if like:
} else if dataType == "formData" || contentType.contains("multipart/form-data"), let boundary = contentType.components(separatedBy: "=").last {It's a bit verbose but it keeps the one form-data case in a single branch. It also won't do the string processing of the boundary until the content type itself has been verified.
There was a problem hiding this comment.
"formData" and "multipart/form-data" call different functions, so doing this would require a second if to call one or the other.
|
Since you didn't like the force unwrap, I've restored the old code that creates a new boundary if none is present. Note that on Android there is no similar code, if there is no boundary it will throw there, but this change is for iOS only, so I'm not changing anything there. |
The changes introduced in ionic-team#7306 fixed several bugs with multipart/form-data requests on iOS. However, the extraction of the actual boundary value might fail due to the value being surrounded by double quotes, which is allowed and happens occasionally. Additionally, the `Content-Type` header might include other keys such as `charset`. This change extracts the boundary value regardless of the order of the individual keys within the `Content-Type` header.
Marking it as breaking since it adds a contentType param to
getRequestDataAsMultipartFormData, which is a public function.Removes the custom boundary
native-bridge.tsis adding and uses the browser one.Read the browser boundary from native and uses that one instead of a custom one (android already does that).
Removes some new lines added that break on some servers.
Makes
getRequestDataFromFormDatapublic since all other methods are public.closes #6748
closes #7242
closes #6832