Skip to content

Commit 06a5da0

Browse files
noahsmartinnoahsmartin
committed
PR feedback
1 parent 0b088cb commit 06a5da0

File tree

7 files changed

+226
-92
lines changed

7 files changed

+226
-92
lines changed

‎src/commands/mobile_app/upload.rs‎

Lines changed: 26 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ use crate::utils::fs::get_sha1_checksums;
2424
use crate::utils::fs::TempFile;
2525
#[cfg(target_os = "macos")]
2626
use crate::utils::mobile_app::handle_asset_catalogs;
27-
use crate::utils::mobile_app::{is_aab_file, is_apk_file, is_apple_app, is_ipa_file, is_zip_file};
27+
use crate::utils::mobile_app::{
28+
ipa_to_xcarchive, is_aab_file, is_apk_file, is_apple_app, is_ipa_file, is_zip_file,
29+
};
2830
use crate::utils::progress::ProgressBar;
2931
use crate::utils::vcs;
3032

@@ -95,12 +97,29 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
9597

9698
let normalized_zip = if path.is_file() {
9799
debug!("Normalizing file: {}", path.display());
98-
normalize_file(path, &byteview).with_context(|| {
99-
format!(
100-
"Failed to generate uploadable bundle for file {}",
101-
path.display()
102-
)
103-
})?
100+
101+
// Handle IPA files by converting them to XCArchive
102+
if is_zip_file(&byteview) && is_ipa_file(&byteview)? {
103+
debug!("Converting IPA file to XCArchive structure");
104+
let temp_dir = crate::utils::fs::TempDir::create()?;
105+
let xcarchive_path =
106+
ipa_to_xcarchive(path, &byteview, &temp_dir).with_context(|| {
107+
format!(
108+
"Failed to convert IPA to XCArchive for file {}",
109+
path.display()
110+
)
111+
})?;
112+
normalize_directory(&xcarchive_path).with_context(|| {
113+
format!("Failed to normalize XCArchive for file {}", path.display())
114+
})?
115+
} else {
116+
normalize_file(path, &byteview).with_context(|| {
117+
format!(
118+
"Failed to generate uploadable bundle for file {}",
119+
path.display()
120+
)
121+
})?
122+
}
104123
} else if path.is_dir() {
105124
debug!("Normalizing directory: {}", path.display());
106125
normalize_directory(path).with_context(|| {
@@ -212,91 +231,10 @@ fn validate_is_mobile_app(path: &Path, bytes: &[u8]) -> Result<()> {
212231
))
213232
}
214233

215-
fn ipa_to_xcarchive(ipa_path: &Path, ipa_bytes: &[u8]) -> Result<TempFile> {
216-
debug!(
217-
"Converting IPA to XCArchive structure: {}",
218-
ipa_path.display()
219-
);
220-
221-
let temp_dir = crate::utils::fs::TempDir::create()?;
222-
let xcarchive_dir = temp_dir.path().join("archive.xcarchive");
223-
let products_dir = xcarchive_dir.join("Products");
224-
let applications_dir = products_dir.join("Applications");
225-
226-
debug!("Creating XCArchive directory structure");
227-
std::fs::create_dir_all(&applications_dir)?;
228-
229-
// Extract IPA file
230-
let cursor = std::io::Cursor::new(ipa_bytes);
231-
let mut ipa_archive = zip::ZipArchive::new(cursor)?;
232-
233-
let mut app_name = String::new();
234-
235-
// Extract .app from Payload/ directory
236-
for i in 0..ipa_archive.len() {
237-
let mut file = ipa_archive.by_index(i)?;
238-
239-
if let Some(stripped) = file.name().strip_prefix("Payload/") {
240-
if let Some(app_folder_name) = stripped.strip_suffix(".app/") {
241-
app_name = app_folder_name.to_string();
242-
debug!("Found app: {}", app_name);
243-
}
244-
245-
if !file.is_dir() {
246-
// Create the file path in the XCArchive structure
247-
let target_path = applications_dir.join(stripped);
248-
249-
// Create parent directories
250-
if let Some(parent) = target_path.parent() {
251-
std::fs::create_dir_all(parent)?;
252-
}
253-
254-
// Extract file
255-
let mut target_file = std::fs::File::create(&target_path)?;
256-
std::io::copy(&mut file, &mut target_file)?;
257-
}
258-
}
259-
}
260-
261-
// Create Info.plist for XCArchive
262-
let info_plist_path = xcarchive_dir.join("Info.plist");
263-
264-
let info_plist_content = format!(
265-
r#"<?xml version="1.0" encoding="UTF-8"?>
266-
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
267-
<plist version="1.0">
268-
<dict>
269-
<key>ApplicationProperties</key>
270-
<dict>
271-
<key>ApplicationPath</key>
272-
<string>Applications/{app_name}.app</string>
273-
</dict>
274-
<key>ArchiveVersion</key>
275-
<integer>1</integer>
276-
</dict>
277-
</plist>"#
278-
);
279-
280-
std::fs::write(&info_plist_path, info_plist_content)?;
281-
282-
debug!(
283-
"Created XCArchive Info.plist at: {}",
284-
info_plist_path.display()
285-
);
286-
normalize_directory(&xcarchive_dir)
287-
}
288-
289234
// For APK and AAB files, we'll copy them directly into the zip
290-
// For IPA files, we'll convert them to XCArchive structure first
291235
fn normalize_file(path: &Path, bytes: &[u8]) -> Result<TempFile> {
292236
debug!("Creating normalized zip for file: {}", path.display());
293237

294-
// Check if this is an IPA file that needs conversion
295-
if is_zip_file(bytes) && is_ipa_file(bytes)? {
296-
debug!("Converting IPA file to XCArchive structure");
297-
return ipa_to_xcarchive(path, bytes);
298-
}
299-
300238
let temp_file = TempFile::create()?;
301239
let mut zip = ZipWriter::new(temp_file.open()?);
302240

‎src/utils/mobile_app/ipa.rs‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
#![cfg(feature = "unstable-mobile-app")]
2+
3+
use anyhow::{anyhow, Result};
4+
use log::debug;
5+
use std::io::Cursor;
6+
use std::path::{Path, PathBuf};
7+
use zip::ZipArchive;
8+
9+
use crate::utils::fs::TempDir;
10+
11+
/// Converts an IPA file to an XCArchive directory structure. The provided IPA must be a valid IPA file.
12+
///
13+
/// # Format Overview
14+
///
15+
/// ## IPA (iOS App Store Package)
16+
/// An IPA file is a compressed archive containing an iOS app ready for distribution.
17+
/// It has the following structure:
18+
/// ```
19+
/// MyApp.ipa
20+
/// └── Payload/
21+
/// └── MyApp.app/
22+
/// ├── Info.plist
23+
/// ├── MyApp (executable)
24+
/// ├── Assets.car
25+
/// └── ... (other app resources)
26+
/// ```
27+
///
28+
/// ## XCArchive (Xcode Archive)
29+
/// An XCArchive is a directory structure created by Xcode when archiving an app for distribution.
30+
/// It has the following structure:
31+
/// ```
32+
/// MyApp.xcarchive/
33+
/// ├── Info.plist
34+
/// ├── Products/
35+
/// │ └── Applications/
36+
/// │ └── MyApp.app/
37+
/// │ ├── Info.plist
38+
/// │ ├── MyApp (executable)
39+
/// │ ├── Assets.car
40+
/// │ └── ... (other app resources)
41+
/// └── ... (other archive metadata)
42+
/// ```
43+
///
44+
/// # Transformation Process
45+
///
46+
/// This function performs the following steps:
47+
/// 1. Creates the XCArchive directory structure (`archive.xcarchive/Products/Applications/`)
48+
/// 2. Extracts the app name from the IPA by finding the shortest path ending with `.app/Info.plist`
49+
/// 3. Extracts all files from the IPA's `Payload/` directory into the XCArchive structure
50+
/// 4. Creates an `Info.plist` file for the XCArchive with the app path reference
51+
/// 5. Returns the path to the XCArchive directory structure
52+
pub fn ipa_to_xcarchive(ipa_path: &Path, ipa_bytes: &[u8], temp_dir: &TempDir) -> Result<PathBuf> {
53+
debug!(
54+
"Converting IPA to XCArchive structure: {}",
55+
ipa_path.display()
56+
);
57+
58+
let xcarchive_dir = temp_dir.path().join("archive.xcarchive");
59+
let products_dir = xcarchive_dir.join("Products");
60+
let applications_dir = products_dir.join("Applications");
61+
62+
debug!("Creating XCArchive directory structure");
63+
std::fs::create_dir_all(&applications_dir)?;
64+
65+
// Extract IPA file
66+
let cursor = Cursor::new(ipa_bytes);
67+
let mut ipa_archive = ZipArchive::new(cursor)?;
68+
69+
let app_name = extract_app_name_from_ipa(&ipa_archive)?;
70+
71+
// Extract all files from the archive
72+
for i in 0..ipa_archive.len() {
73+
let mut file = ipa_archive.by_index(i)?;
74+
75+
if let Some(name) = file.enclosed_name() {
76+
if let Ok(stripped) = name.strip_prefix("Payload/") {
77+
if !file.is_dir() {
78+
// Create the file path in the XCArchive structure
79+
let target_path = applications_dir.join(stripped);
80+
81+
// Create parent directories if necessary
82+
if let Some(parent) = target_path.parent() {
83+
std::fs::create_dir_all(parent)?;
84+
}
85+
86+
// Extract file
87+
let mut target_file = std::fs::File::create(&target_path)?;
88+
std::io::copy(&mut file, &mut target_file)?;
89+
}
90+
}
91+
}
92+
}
93+
94+
// Create Info.plist for XCArchive
95+
let info_plist_path = xcarchive_dir.join("Info.plist");
96+
97+
let info_plist_content = format!(
98+
r#"<?xml version="1.0" encoding="UTF-8"?>
99+
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
100+
<plist version="1.0">
101+
<dict>
102+
<key>ApplicationProperties</key>
103+
<dict>
104+
<key>ApplicationPath</key>
105+
<string>Applications/{app_name}.app</string>
106+
</dict>
107+
<key>ArchiveVersion</key>
108+
<integer>1</integer>
109+
</dict>
110+
</plist>"#
111+
);
112+
113+
std::fs::write(&info_plist_path, info_plist_content)?;
114+
115+
debug!(
116+
"Created XCArchive Info.plist at: {}",
117+
info_plist_path.display()
118+
);
119+
Ok(xcarchive_dir)
120+
}
121+
122+
fn extract_app_name_from_ipa(archive: &ZipArchive<Cursor<&[u8]>>) -> Result<String> {
123+
archive
124+
.file_names()
125+
.filter(|name| name.starts_with("Payload/") && name.ends_with(".app/Info.plist"))
126+
.min_by_key(|name| name.len())
127+
.and_then(|name| name.strip_prefix("Payload/"))
128+
.and_then(|name| name.split('/').next())
129+
.and_then(|name| name.strip_suffix(".app"))
130+
.map(|name| name.to_owned())
131+
.ok_or_else(|| anyhow!("No .app found in IPA"))
132+
}

‎src/utils/mobile_app/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22

33
#[cfg(target_os = "macos")]
44
mod apple;
5+
mod ipa;
56
mod validation;
67

78
#[cfg(target_os = "macos")]
89
pub use self::apple::handle_asset_catalogs;
10+
pub use self::ipa::ipa_to_xcarchive;
911
pub use self::validation::{is_aab_file, is_apk_file, is_apple_app, is_ipa_file, is_zip_file};

‎src/utils/mobile_app/validation.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,13 @@ pub fn is_aab_file(bytes: &[u8]) -> Result<bool> {
3939

4040
pub fn is_ipa_file(bytes: &[u8]) -> Result<bool> {
4141
let cursor = std::io::Cursor::new(bytes);
42-
let mut archive = zip::ZipArchive::new(cursor)?;
42+
let archive = zip::ZipArchive::new(cursor)?;
4343

44-
let is_ipa = archive
45-
.file_names()
46-
.any(|name| name.starts_with("Payload/") && name.ends_with(".app/"));
44+
let is_ipa = archive.file_names().any(|name| {
45+
name.starts_with("Payload/")
46+
&& name.ends_with(".app/Info.plist")
47+
&& name.matches('/').count() == 2
48+
});
4749

4850
Ok(is_ipa)
4951
}

‎tests/integration/_cases/mobile_app/mobile_app-upload-ipa.trycmd‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
```
2+
$ sentry-cli mobile-app upload tests/integration/_fixtures/mobile_app/ipa.ipa --sha test_sha
3+
? success
4+
Successfully uploaded 1 file to Sentry
5+
- tests/integration/_fixtures/mobile_app/ipa.ipa
6+
7+
```

‎tests/integration/_fixtures/mobile_app/ipa.ipa‎

24.2 KB
Binary file not shown.

‎tests/integration/mobile_app/upload.rs‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,3 +172,56 @@ fn command_mobile_app_upload_apk_chunked() {
172172
.register_trycmd_test("mobile_app/mobile_app-upload-apk.trycmd")
173173
.with_default_token();
174174
}
175+
176+
#[test]
177+
/// This test simulates a full chunk upload for an IPA file (with only one chunk).
178+
/// It verifies that the Sentry CLI makes the expected API calls to the chunk upload endpoint
179+
/// and that the data sent to the chunk upload endpoint is exactly as expected.
180+
/// It also verifies that the correct calls are made to the assemble endpoint.
181+
fn command_mobile_app_upload_ipa_chunked() {
182+
let is_first_assemble_call = AtomicBool::new(true);
183+
184+
TestManager::new()
185+
.mock_endpoint(
186+
MockEndpointBuilder::new("GET", "/api/0/organizations/wat-org/chunk-upload/")
187+
.with_response_file("mobile_app/get-chunk-upload.json"),
188+
)
189+
.mock_endpoint(
190+
MockEndpointBuilder::new("POST", "/api/0/organizations/wat-org/chunk-upload/")
191+
.with_response_fn(move |request| {
192+
let content_type_headers = request.header("content-type");
193+
assert_eq!(
194+
content_type_headers.len(),
195+
1,
196+
"content-type header should be present exactly once, found {} times",
197+
content_type_headers.len()
198+
);
199+
vec![] // Client does not expect a response body
200+
}),
201+
)
202+
.mock_endpoint(
203+
MockEndpointBuilder::new(
204+
"POST",
205+
"/api/0/projects/wat-org/wat-project/files/preprodartifacts/assemble/",
206+
)
207+
.with_header_matcher("content-type", "application/json")
208+
.with_matcher(r#"{"checksum":"ed9da71e3688261875db21b266da84ffe004a8a4","chunks":["ed9da71e3688261875db21b266da84ffe004a8a4"],"git_sha":"test_sha"}"#)
209+
.with_response_fn(move |_| {
210+
if is_first_assemble_call.swap(false, Ordering::Relaxed) {
211+
r#"{
212+
"state": "created",
213+
"missingChunks": ["ed9da71e3688261875db21b266da84ffe004a8a4"]
214+
}"#
215+
} else {
216+
r#"{
217+
"state": "ok",
218+
"missingChunks": []
219+
}"#
220+
}
221+
.into()
222+
})
223+
.expect(2),
224+
)
225+
.register_trycmd_test("mobile_app/mobile_app-upload-ipa.trycmd")
226+
.with_default_token();
227+
}

0 commit comments

Comments
 (0)