Skip to content

Commit

Permalink
cli: determine archive type based on filename instead of path (micros…
Browse files Browse the repository at this point in the history
…oft#224602)

Refs microsoft#219632

Seems like PRSS sometimes(?) doesn't return the full archive name in
the response. I don't reproduce this, but others consistently do. This
PR removes the dependency on the URL path and instead checks for the
gzip magic number in the first two bytes of the archive to figure out
what to do.
  • Loading branch information
connor4312 committed Aug 2, 2024
1 parent 331b772 commit fcfd4be
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 20 deletions.
15 changes: 8 additions & 7 deletions cli/src/update_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

use std::{ffi::OsStr, fmt, path::Path};
use std::{fmt, path::Path};

use serde::{Deserialize, Serialize};

use crate::{
constants::VSCODE_CLI_UPDATE_ENDPOINT,
debug, log, options, spanf,
util::{
errors::{AnyError, CodeError, WrappedError},
errors::{wrap, AnyError, CodeError, WrappedError},
http::{BoxedHttp, SimpleResponse},
io::ReportCopyProgress,
tar, zipper,
tar::{self, has_gzip_header},
zipper,
},
};

Expand Down Expand Up @@ -178,10 +179,10 @@ pub fn unzip_downloaded_release<T>(
where
T: ReportCopyProgress,
{
if compressed_file.extension() == Some(OsStr::new("zip")) {
zipper::unzip_file(compressed_file, target_dir, reporter)
} else {
tar::decompress_tarball(compressed_file, target_dir, reporter)
match has_gzip_header(compressed_file) {
Ok((f, true)) => tar::decompress_tarball(f, target_dir, reporter),
Ok((f, false)) => zipper::unzip_file(f, target_dir, reporter),
Err(e) => Err(wrap(e, "error checking for gzip header")),
}
}

Expand Down
21 changes: 14 additions & 7 deletions cli/src/util/tar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
use crate::util::errors::{wrap, WrappedError};

use flate2::read::GzDecoder;
use std::fs;
use std::io::Seek;
use std::fs::{self, File};
use std::io::{Read, Seek};
use std::path::{Path, PathBuf};
use tar::Archive;

Expand Down Expand Up @@ -57,16 +57,13 @@ fn should_skip_first_segment(file: &fs::File) -> Result<(bool, u64), WrappedErro
}

pub fn decompress_tarball<T>(
path: &Path,
mut tar_gz: File,
parent_path: &Path,
mut reporter: T,
) -> Result<(), WrappedError>
where
T: ReportCopyProgress,
{
let mut tar_gz = fs::File::open(path)
.map_err(|e| wrap(e, format!("error opening file {}", path.display())))?;

let (skip_first, num_entries) = should_skip_first_segment(&tar_gz)?;
let report_progress_every = num_entries / 20;
let mut entries_so_far = 0;
Expand All @@ -81,7 +78,7 @@ where
let mut archive = Archive::new(tar);
archive
.entries()
.map_err(|e| wrap(e, format!("error opening archive {}", path.display())))?
.map_err(|e| wrap(e, "error opening archive"))?
.filter_map(|e| e.ok())
.try_for_each::<_, Result<_, WrappedError>>(|mut entry| {
// approximate progress based on where we are in the archive:
Expand Down Expand Up @@ -118,3 +115,13 @@ where

Ok(())
}

pub fn has_gzip_header(path: &Path) -> std::io::Result<(File, bool)> {
let mut file = fs::File::open(path)?;
let mut header = [0; 2];
let _ = file.read_exact(&mut header);

file.rewind()?;

Ok((file, header[0] == 0x1f && header[1] == 0x8b))
}
9 changes: 3 additions & 6 deletions cli/src/util/zipper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,12 @@ fn should_skip_first_segment(archive: &mut ZipArchive<File>) -> bool {
archive.len() > 1 // prefix removal is invalid if there's only a single file
}

pub fn unzip_file<T>(path: &Path, parent_path: &Path, mut reporter: T) -> Result<(), WrappedError>
pub fn unzip_file<T>(file: File, parent_path: &Path, mut reporter: T) -> Result<(), WrappedError>
where
T: ReportCopyProgress,
{
let file = fs::File::open(path)
.map_err(|e| wrap(e, format!("unable to open file {}", path.display())))?;

let mut archive = zip::ZipArchive::new(file)
.map_err(|e| wrap(e, format!("failed to open zip archive {}", path.display())))?;
let mut archive =
zip::ZipArchive::new(file).map_err(|e| wrap(e, "failed to open zip archive"))?;

let skip_segments_no = usize::from(should_skip_first_segment(&mut archive));
let report_progress_every = archive.len() / 20;
Expand Down

0 comments on commit fcfd4be

Please sign in to comment.