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

[fastlane_core] use randomly generated filename(s) for -assetFile when uploading binaries to Testflight/App Store #19716

Merged
merged 6 commits into from
Jan 13, 2022

Conversation

dokimyj
Copy link
Contributor

@dokimyj dokimyj commented Dec 20, 2021

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

Resolves #19708

When trying to upload ipa file to Testflight / App Store, the upload process fails if the name of the ipa file is in multibytes.
(The error message shows the name of the ipa/pkg file as blank.)

You can reproduce the error by building your app with output_name: <multibytechar>.ipa and trying uploading it to Testflight / App Store.

This changed behavior would make deliver/pilot use .ipa/.pkg derived from copy_ipa/copy_pkg methods, which will be found from the package_path: .itmsp.

If there is no .ipa/.pkg file in .itmsp path, deliver/pilot will use upload_ipa/upload_pkg as is.

copy_ipa
copy_pkg

Description

  • added SecureRandom to randomize the name of asset file(s) to be uploaded.
  • File(s) to be uploaded shall be renamed randomly with ↑SecureRandom if asset_path exists and .itmsp is not forced.
  • Tests fixed for randomized uploads(Thanks @joshdholtz !)

Acceptance test result: #19716 (comment)

Obsolete explanations (for fixing deliver/pilot collectively)
  • For deliver/runner.rb, replaced the value of variable ipa_path/pkg_path with the one(s) in the itmsp file.
  • For pilot/build_manager, replaced the value of variable options[:ipa]/options[:pkg] the same way above.
    • added upload_ipa/upload_pkg, not to change the values inside the hash options.
  • Both changes would be restored to options[:ipa/pkg] values if there is no file for asset_file in .itmsp.

I tested the way I could reproduce the problem the related issue undergone, and succeeded upload_to_testflight.

00:08:23 path/to/ipa/file/alphanumeric아님.ipa
00:08:23 ----------------------------------
00:08:23 --- Step: upload_to_testflight ---
00:08:23 ----------------------------------
00:08:23 Creating authorization token for App Store Connect API
00:08:27 Ready to upload new build to TestFlight (App: <someRandomNumber>)...
00:08:28 Going to upload updated app to App Store Connect
00:08:28 This might take a few minutes. Please don't interrupt the script.
00:10:58 iTunes Transporter successfully finished its job
00:10:58 --------------------------------------------------------------------------------
00:10:58 Successfully uploaded package to App Store Connect. It might take a few minutes until it's visible online.
00:10:58 --------------------------------------------------------------------------------
00:10:58 Successfully uploaded the new binary to App Store Connect

Testing Steps

bundle exec rspec fastlane_core

…aries with non-alphanumeric filename to Testflight/App Store
[deliver][pilot] use copied ipa/pkg for -assetFile when uploading binaries with non-alphanumeric filename to Testflight/App Store
@dokimyj dokimyj changed the title [fastlane_core] use copied ipa/pkg for -assetFile when uploading binaries with non-alphanumeric filename to Testflight/App Store [deliver][pilot] use copied ipa/pkg for -assetFile when uploading binaries with non-alphanumeric filename to Testflight/App Store Dec 20, 2021
@dokimyj dokimyj changed the title [deliver][pilot] use copied ipa/pkg for -assetFile when uploading binaries with non-alphanumeric filename to Testflight/App Store [deliver][pilot] use copied ipa/pkg for -assetFile when uploading binaries to Testflight/App Store Dec 20, 2021
Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

I did not realize this side effect 😱 I'm sorry for the issue!

I think instead of changing this in deliver and pilot, we could probably change this in https://github.com/fastlane/fastlane/blob/master/fastlane_core/lib/fastlane_core/itunes_transporter.rb#L504-L506

tmp_asset_path = File.join(Dir.tmpdir, File.basename(asset_path))

to

require 'securerandom' # place at top of file

new_file_name = "#{SecureRandom.uuid}#{File.extname(asset_path)}"
tmp_asset_path = File.join(Dir.tmpdir, new_file_name)

I didn't run this but I think this should do it 🙃

@dokimyj
Copy link
Contributor Author

dokimyj commented Jan 5, 2022

@joshdholtz Thanks for the comment! I'll take this change and test to see how it changes the behavior in upload_to_testflight.

@joshdholtz
Copy link
Member

@dokimyj Perfect! Let me know if you need anything from me 😊 Thanks!

@dokimyj dokimyj changed the title [deliver][pilot] use copied ipa/pkg for -assetFile when uploading binaries to Testflight/App Store [fastlane_core] use copied ipa/pkg for -assetFile when uploading binaries to Testflight/App Store Jan 6, 2022
@dokimyj dokimyj changed the title [fastlane_core] use copied ipa/pkg for -assetFile when uploading binaries to Testflight/App Store [fastlane_core] use randomly generated filename(s) -assetFile when uploading binaries to Testflight/App Store Jan 6, 2022
@dokimyj
Copy link
Contributor Author

dokimyj commented Jan 6, 2022

Changed Fastlane core part instead(itms transporting portion) and it works perfect!

It seems far more clear & works perfect!

Logs for `upload_to_testflight`
00:08:00 INFO [2022-01-06 13:15:47.19]: Successfully exported and compressed dSYM file
00:08:01 INFO [2022-01-06 13:15:47.20]: Successfully exported and signed the ipa file:
00:08:01 INFO [2022-01-06 13:15:47.20]: path/to/ipa/Not알파누메릭.ipa
00:08:01 -------
00:08:01 INFO [2022-01-06 13:15:47.23]: ----------------------------------
00:08:01 INFO [2022-01-06 13:15:47.23]: --- Step: upload_to_testflight ---
00:08:01 INFO [2022-01-06 13:15:47.23]: ----------------------------------
00:08:01 INFO [2022-01-06 13:15:47.40]: Creating authorization token for App Store Connect API
00:08:01 DEBUG [2022-01-06 13:15:47.41]: App identifier (foo.bar.baz.test.app)
00:08:05 INFO [2022-01-06 13:15:52.16]: Ready to upload new build to TestFlight (App: <some random number>)...
00:08:05 DEBUG [2022-01-06 13:15:52.17]: App Platform (ios)
00:08:06 INFO [2022-01-06 13:15:52.87]: Wrote XML data to '/var/folders/hn/z40t3mf96j7_s80gbflmhp300000gq/T/d20220106-65095-1jvkj2y/1194487182-9501566f-3b72-4f3a-9ee8-e273bd243fb1.itmsp'
00:08:06 INFO [2022-01-06 13:15:52.93]: Going to upload updated app to App Store Connect
00:08:06 INFO [2022-01-06 13:15:52.93]: This might take a few minutes. Please don't interrupt the script.
00:08:06 DEBUG [2022-01-06 13:15:52.93]: xcrun iTMSTransporter -m upload -jwt YourJWT -assetFile /var/folders/hn/z40t3mf96j7_s80gbflmhp300000gq/T/8c5bf74c-efc2-4feb-a51e-54284b68454a.ipa -k 100000 2>&1

(Very long Testflight upload logs marked as `INFO` or `DEBUG`)

00:09:11 DEBUG [2022-01-06 13:16:57.92]: [Transporter]: INFO: Done performing uploadDone notification to Apple.
00:09:11 DEBUG [2022-01-06 13:16:57.94]: [Transporter]: INFO: Transporter's command line arguments are: -m upload -jwt **hidden value** -assetFile /var/folders/hn/z40t3mf96j7_s80gbflmhp300000gq/T/8c5bf74c-efc2-4feb-a51e-54284b68454a.ipa -k 100000
00:09:11 DEBUG [2022-01-06 13:16:57.94]: [Transporter]: INFO: The package: /var/folders/hn/z40t3mf96j7_s80gbflmhp300000gq/T/DeveloperAPIUpload3993524694640234644/<some random number>.itmsp has been successfully uploaded.
00:09:11 INFO [2022-01-06 13:16:58.01]: iTunes Transporter successfully finished its job
00:09:11 DEBUG [2022-01-06 13:16:58.01]: [Transporter]: DBG-X: Returning 0
00:09:11 INFO [2022-01-06 13:16:58.17]: -----------------------------------------------------
00:09:11 INFO [2022-01-06 13:16:58.17]: Successfully uploaded package to App Store Connect. It might take a few minutes until it's visible online.
00:09:11 INFO [2022-01-06 13:16:58.17]: -----------------------------------------------------
00:09:11 INFO [2022-01-06 13:16:58.18]: Successfully uploaded the new binary to App Store Connect
00:09:11 WARN [2022-01-06 13:16:58.18]: `skip_waiting_for_build_processing` used and no `changelog` supplied - skipping waiting for build processing

and Thanks for correcting tests! I thought I gotta change the code to work only when non-alphanumeric filename comes.

@dokimyj dokimyj changed the title [fastlane_core] use randomly generated filename(s) -assetFile when uploading binaries to Testflight/App Store [fastlane_core] use randomly generated filename(s) for -assetFile when uploading binaries to Testflight/App Store Jan 6, 2022
Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

Thanks for finding this issue and debugging this! Excited to get this one out 😊

@fastlane-bot
Copy link

Congratulations! 🎉 This was released as part of fastlane 2.200.0 🚀

@fastlane fastlane locked and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing to upload an IPA which the name is set multibyte characters
3 participants