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

[GITHUB-10139][Fix] Generated Swift code not working for multipart/form-data video upload #714

Conversation

aman-v-singh
Copy link
Contributor

@aman-v-singh aman-v-singh commented Jul 5, 2023

Context

  • In postman-app-support, Issue #10139 The user has described an issue they were facing with Swift code generated for Content-Type multipart/form-data.

Root-Cause-Analysis

  • When the user was using the generated Swift code for uploading a .MOV video file, they were facing an issue on reaching this line:-
    let fileContent = String(data: fileData, encoding: .utf8)!

  • On analysis, it is found that the error is arising because the generated code is trying to encode video data in UTF-8 format to be converted into String type, but we must realise every character in fileData would not be encodable in UTF-8 format.

Solution

  • Instead of converting file data to String type, we will change the body variable from String to Data type. This way we don't have to convert the fileContent to String.

  • For details refer to this answer.

Code Generated

import Foundation
#if canImport(FoundationNetworking)
import FoundationNetworking
#endif

let parameters = [
    [
        "key": "VideoFile",
        "src": "{VIDEO_FILE_PATH}",
        "type": "file"
    ]] as [[String: Any]]

let boundary = "Boundary-\(UUID().uuidString)"
var body = Data()
var error: Error? = nil
for param in parameters {
    if param["disabled"] != nil { continue }
    let paramName = param["key"]!
    body += Data("--\(boundary)\r\n".utf8)
    body += Data("Content-Disposition:form-data; name=\"\(paramName)\"".utf8)
    if param["contentType"] != nil {
        body += Data("\r\nContent-Type: \(param["contentType"] as! String)".utf8)
    }
    let paramType = param["type"] as! String
    if paramType == "text" {
        let paramValue = param["value"] as! String
        body += Data("\r\n\r\n\(paramValue)\r\n".utf8)
    } else {
        let paramSrc = param["src"] as! String
        let fileURL = URL(fileURLWithPath: paramSrc)
        if let fileContent = try? Data(contentsOf: fileURL) {
            body += Data("; filename=\"\(paramSrc)\"\r\n".utf8)
            body += Data("Content-Type: \"content-type header\"\r\n".utf8)
            body += Data("\r\n".utf8)
            body += fileContent
            body += Data("\r\n".utf8)
        }
    }
}
body += Data("--\(boundary)--\r\n".utf8);
let postData = body


var request = URLRequest(url: URL(string: "https://postman-echo.com/post")!,timeoutInterval: Double.infinity)
request.addValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type")

request.httpMethod = "POST"
request.httpBody = postData

let task = URLSession.shared.dataTask(with: request) { data, response, error in 
    guard let data = data else {
        print(String(describing: error))
        exit(EXIT_SUCCESS)
    }
    print(String(data: data, encoding: .utf8)!)
    exit(EXIT_SUCCESS)
}

task.resume()
dispatchMain()

Copy link
Member

@VShingala VShingala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aman-v-singh Have we tested that this change doesn't affect existing flow. i.e. Other files which are in text format can sill be sent correctly in form-data body?

@VShingala
Copy link
Member

@aman-v-singh Could you also attach updated snippet here in description once? Also let's communicate this latest snippet with the User that reported this issue and make sure it works for them to get their feedback.

@aman-v-singh
Copy link
Contributor Author

@VShingala I have added the newly generated code above in the description and notified the user of the same.

@aman-v-singh Have we tested that this change doesn't affect existing flow. i.e. Other files which are in text format can sill be sent correctly in form-data body?

The Newman tests take care of these different possibilities.
eg:- Single/multiple file upload via form-data and Binary file upload via form-data check for different types of files.

@VShingala
Copy link
Member

@aman-v-singh There is a bit of catch with forma-data type of bodies and how files are stored in collection if selected. For example, the collection that you linked contains path to a file which is for the user that created this collection. This file doesn't exist in pipelines system or repository anywhere. As such actual data within files is not being sent with the request.

Which is why just wanted to make sure this is working correctly, could you give it a try manually and verify once?

image

@dhwaneetbhatt
Copy link
Contributor

@aman-v-singh There is a bit of catch with forma-data type of bodies and how files are stored in collection if selected. For example, the collection that you linked contains path to a file which is for the user that created this collection. This file doesn't exist in pipelines system or repository anywhere. As such actual data within files is not being sent with the request.

Which is why just wanted to make sure this is working correctly, could you give it a try manually and verify once?

image

@VShingala these files and paths are generated on the fly when test case runs and this path gets updated locally. We usually don't commit the changes because it depends on the machine being run.

@VShingala
Copy link
Member

@dhwaneetbhatt Thanks! Checked this out and Didn't know about this till now. This was a practice of habit for me while I was working on this as this file manipulation did not exist. 😅

For ref, below is code responsible for manipulation.

@aman-v-singh aman-v-singh merged commit 89f2f71 into develop Jul 7, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants