From f276140d0fc0d0f0be8bce3356d69389eb8ab914 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Fri, 15 Nov 2019 15:07:00 -0800 Subject: [PATCH] cli-test: a tool for testing command-line programs. Not looking for other users right now, this is just enough to test unzip/zip/zipinfo. This includes tests for unzip and ziptool, along with a change to unzip's behavior to fix AOSP `make dist` when using ziptool unzip. Also add the boilerplate to run these tests on the device, in presubmit. Fix command name in --help output. Test: atest ziptool-tests Change-Id: I5c0215a3ab8cb2cd5fc517ed9c188f81a7bf4520 --- cli-test/.clang-format | 1 + cli-test/Android.bp | 7 + cli-test/README.md | 90 +++++ cli-test/cli-test.cpp | 320 ++++++++++++++++++ libziparchive/Android.bp | 12 + libziparchive/cli-tests/files/example.zip | Bin 0 -> 1082 bytes libziparchive/cli-tests/unzip.test | 148 ++++++++ libziparchive/cli-tests/zipinfo.test | 53 +++ libziparchive/run-ziptool-tests-on-android.sh | 15 + libziparchive/unzip.cpp | 26 +- libziparchive/ziptool-tests.xml | 26 ++ 11 files changed, 688 insertions(+), 10 deletions(-) create mode 120000 cli-test/.clang-format create mode 100644 cli-test/Android.bp create mode 100644 cli-test/README.md create mode 100644 cli-test/cli-test.cpp create mode 100644 libziparchive/cli-tests/files/example.zip create mode 100755 libziparchive/cli-tests/unzip.test create mode 100755 libziparchive/cli-tests/zipinfo.test create mode 100755 libziparchive/run-ziptool-tests-on-android.sh create mode 100644 libziparchive/ziptool-tests.xml diff --git a/cli-test/.clang-format b/cli-test/.clang-format new file mode 120000 index 000000000..fd0645fdf --- /dev/null +++ b/cli-test/.clang-format @@ -0,0 +1 @@ +../.clang-format-2 \ No newline at end of file diff --git a/cli-test/Android.bp b/cli-test/Android.bp new file mode 100644 index 000000000..37a1d1b37 --- /dev/null +++ b/cli-test/Android.bp @@ -0,0 +1,7 @@ +cc_binary { + name: "cli-test", + host_supported: true, + srcs: ["cli-test.cpp"], + cflags: ["-Wall", "-Werror"], + shared_libs: ["libbase"], +} diff --git a/cli-test/README.md b/cli-test/README.md new file mode 100644 index 000000000..643eb7456 --- /dev/null +++ b/cli-test/README.md @@ -0,0 +1,90 @@ +# cli-test + +## What? + +`cli-test` makes integration testing of command-line tools easier. + +## Goals + +* Readable syntax. Common cases should be concise, and pretty much anyone + should be able to read tests even if they've never seen this tool before. + +* Minimal issues with quoting. The toybox tests -- being shell scripts -- + quickly become a nightmare of quoting. Using a non ad hoc format (such as + JSON) would have introduced similar but different quoting issues. A custom + format, while annoying, side-steps this. + +* Sensible defaults. We expect your exit status to be 0 unless you say + otherwise. We expect nothing on stderr unless you say otherwise. And so on. + +* Convention over configuration. Related to sensible defaults, we don't let you + configure things that aren't absolutely necessary. So you can't keep your test + data anywhere except in the `files/` subdirectory of the directory containing + your test, for example. + +## Non Goals + +* Portability. Just being able to run on Linux (host and device) is sufficient + for our needs. macOS is probably easy enough if we ever need it, but Windows + probably doesn't make sense. + +## Syntax + +Any all-whitespace line, or line starting with `#` is ignored. + +A test looks like this: +``` +name: unzip -l +command: unzip -l $FILES/example.zip d1/d2/x.txt +after: [ ! -f d1/d2/x.txt ] +expected-stdout: + Archive: $FILES/example.zip + Length Date Time Name + --------- ---------- ----- ---- + 1024 2017-06-04 08:45 d1/d2/x.txt + --------- ------- + 1024 1 file +--- +``` + +The `name:` line names the test, and is only for human consumption. + +The `command:` line is the command to be run. Additional commands can be +supplied as zero or more `before:` lines (run before `command:`) and zero or +more `after:` lines (run after `command:`). These are useful for both +setup/teardown but also for testing post conditions (as in the example above). + +Any `command:`, `before:`, or `after:` line is expected to exit with status 0. +Anything else is considered a test failure. + +The `expected-stdout:` line is followed by zero or more tab-prefixed lines that +are otherwise the exact output expected from the command. (There's magic behind +the scenes to rewrite the test files directory to `$FILES` because otherwise any +path in the output would depend on the temporary directory used to run the test.) + +There is currently no `expected-stderr:` line. Standard error is implicitly +expected to be empty, and any output will cause a test failure. (The support is +there, but not wired up because we haven't needed it yet.) + +The fields can appear in any order, but every test must contain at least a +`name:` line and a `command:` line. + +## Output + +The output is intended to resemble gtest. + +## Future Directions + +* It's often useful to be able to *match* against stdout/stderr/a file rather + than give exact expected output. We might want to add explicit support for + this. In the meantime, it's possible to use an `after:` with `grep -q` if + you redirect in your `command:`. + +* In addition to using a `before:` (which will fail a test), it can be useful + to be able to specify tests that would cause us to *skip* a test. An example + would be "am I running as root?". + +* It might be useful to be able to make exit status assertions other than 0? + +* There's currently no way (other than the `files/` directory) to share repeated + setup between tests. diff --git a/cli-test/cli-test.cpp b/cli-test/cli-test.cpp new file mode 100644 index 000000000..d6e27ee3b --- /dev/null +++ b/cli-test/cli-test.cpp @@ -0,0 +1,320 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include +#include +#include + +// Example: + +// name: unzip -n +// before: mkdir -p d1/d2 +// before: echo b > d1/d2/a.txt +// command: unzip -q -n $FILES/zip/example.zip d1/d2/a.txt && cat d1/d2/a.txt +// expected-stdout: +// b + +struct Test { + std::string test_filename; + std::string name; + std::string command; + std::vector befores; + std::vector afters; + std::string expected_stdout; + std::string expected_stderr; + int exit_status = 0; +}; + +static const char* g_progname; +static bool g_verbose; + +static const char* g_file; +static size_t g_line; + +enum Color { kRed, kGreen }; + +static void Print(Color c, const char* lhs, const char* fmt, ...) { + va_list ap; + va_start(ap, fmt); + if (isatty(0)) printf("%s", (c == kRed) ? "\e[31m" : "\e[32m"); + printf("%s%s", lhs, isatty(0) ? "\e[0m" : ""); + vfprintf(stdout, fmt, ap); + putchar('\n'); + va_end(ap); +} + +static void Die(int error, const char* fmt, ...) { + va_list ap; + va_start(ap, fmt); + fprintf(stderr, "%s: ", g_progname); + vfprintf(stderr, fmt, ap); + if (error != 0) fprintf(stderr, ": %s", strerror(error)); + fprintf(stderr, "\n"); + va_end(ap); + _exit(1); +} + +static void V(const char* fmt, ...) { + if (!g_verbose) return; + + va_list ap; + va_start(ap, fmt); + fprintf(stderr, " - "); + vfprintf(stderr, fmt, ap); + fprintf(stderr, "\n"); + va_end(ap); +} + +static void SetField(const char* what, std::string* field, std::string_view value) { + if (!field->empty()) { + Die(0, "%s:%zu: %s already set to '%s'", g_file, g_line, what, field->c_str()); + } + field->assign(value); +} + +// Similar to ConsumePrefix, but also trims, so "key:value" and "key: value" +// are equivalent. +static bool Match(std::string* s, const std::string& prefix) { + if (!android::base::StartsWith(*s, prefix)) return false; + s->assign(android::base::Trim(s->substr(prefix.length()))); + return true; +} + +static void CollectTests(std::vector* tests, const char* test_filename) { + std::string absolute_test_filename; + if (!android::base::Realpath(test_filename, &absolute_test_filename)) { + Die(errno, "realpath '%s'", test_filename); + } + + std::string content; + if (!android::base::ReadFileToString(test_filename, &content)) { + Die(errno, "couldn't read '%s'", test_filename); + } + + size_t count = 0; + g_file = test_filename; + g_line = 0; + auto lines = android::base::Split(content, "\n"); + std::unique_ptr test(new Test); + while (g_line < lines.size()) { + auto line = lines[g_line++]; + if (line.empty() || line[0] == '#') continue; + + if (line[0] == '-') { + if (test->name.empty() || test->command.empty()) { + Die(0, "%s:%zu: each test requires both a name and a command", g_file, g_line); + } + test->test_filename = absolute_test_filename; + tests->push_back(*test.release()); + test.reset(new Test); + ++count; + } else if (Match(&line, "name:")) { + SetField("name", &test->name, line); + } else if (Match(&line, "command:")) { + SetField("command", &test->command, line); + } else if (Match(&line, "before:")) { + test->befores.push_back(line); + } else if (Match(&line, "after:")) { + test->afters.push_back(line); + } else if (Match(&line, "expected-stdout:")) { + // Collect tab-indented lines. + std::string text; + while (g_line < lines.size() && !lines[g_line].empty() && lines[g_line][0] == '\t') { + text += lines[g_line++].substr(1) + "\n"; + } + SetField("expected stdout", &test->expected_stdout, text); + } else { + Die(0, "%s:%zu: syntax error: \"%s\"", g_file, g_line, line.c_str()); + } + } + if (count == 0) Die(0, "no tests found in '%s'", g_file); +} + +static const char* Plural(size_t n) { + return (n == 1) ? "" : "s"; +} + +static std::string ExitStatusToString(int status) { + if (WIFSIGNALED(status)) { + return android::base::StringPrintf("was killed by signal %d (%s)", WTERMSIG(status), + strsignal(WTERMSIG(status))); + } + if (WIFSTOPPED(status)) { + return android::base::StringPrintf("was stopped by signal %d (%s)", WSTOPSIG(status), + strsignal(WSTOPSIG(status))); + } + return android::base::StringPrintf("exited with status %d", WEXITSTATUS(status)); +} + +static bool RunCommands(const char* what, const std::vector& commands) { + bool result = true; + for (auto& command : commands) { + V("running %s \"%s\"", what, command.c_str()); + int exit_status = system(command.c_str()); + if (exit_status != 0) { + result = false; + fprintf(stderr, "Command (%s) \"%s\" %s\n", what, command.c_str(), + ExitStatusToString(exit_status).c_str()); + } + } + return result; +} + +static bool CheckOutput(const char* what, std::string actual_output, + const std::string& expected_output, const std::string& FILES) { + // Rewrite the output to reverse any expansion of $FILES. + actual_output = android::base::StringReplace(actual_output, FILES, "$FILES", true); + + bool result = (actual_output == expected_output); + if (!result) { + fprintf(stderr, "Incorrect %s.\nExpected:\n%s\nActual:\n%s\n", what, expected_output.c_str(), + actual_output.c_str()); + } + return result; +} + +static int RunTests(const std::vector& tests) { + std::vector failures; + + Print(kGreen, "[==========]", " Running %zu tests.", tests.size()); + android::base::Timer total_timer; + for (const auto& test : tests) { + bool failed = false; + + Print(kGreen, "[ RUN ]", " %s", test.name.c_str()); + android::base::Timer test_timer; + + // Set $FILES for this test. + std::string FILES = android::base::Dirname(test.test_filename) + "/files"; + V("setenv(\"FILES\", \"%s\")", FILES.c_str()); + setenv("FILES", FILES.c_str(), 1); + + // Make a safe space to run the test. + TemporaryDir td; + V("chdir(\"%s\")", td.path); + if (chdir(td.path)) Die(errno, "chdir(\"%s\")", td.path); + + // Perform any setup specified for this test. + if (!RunCommands("before", test.befores)) failed = true; + + if (!failed) { + V("running command \"%s\"", test.command.c_str()); + CapturedStdout test_stdout; + CapturedStderr test_stderr; + int exit_status = system(test.command.c_str()); + test_stdout.Stop(); + test_stderr.Stop(); + + V("exit status %d", exit_status); + if (exit_status != test.exit_status) { + failed = true; + fprintf(stderr, "Incorrect exit status: expected %d but %s\n", test.exit_status, + ExitStatusToString(exit_status).c_str()); + } + + if (!CheckOutput("stdout", test_stdout.str(), test.expected_stdout, FILES)) failed = true; + if (!CheckOutput("stderr", test_stderr.str(), test.expected_stderr, FILES)) failed = true; + + if (!RunCommands("after", test.afters)) failed = true; + } + + std::stringstream duration; + duration << test_timer; + if (failed) { + failures.push_back(test.name); + Print(kRed, "[ FAILED ]", " %s (%s)", test.name.c_str(), duration.str().c_str()); + } else { + Print(kGreen, "[ OK ]", " %s (%s)", test.name.c_str(), duration.str().c_str()); + } + } + + // Summarize the whole run and explicitly list all the failures. + + std::stringstream duration; + duration << total_timer; + Print(kGreen, "[==========]", " %zu tests ran. (%s total)", tests.size(), duration.str().c_str()); + + size_t fail_count = failures.size(); + size_t pass_count = tests.size() - fail_count; + Print(kGreen, "[ PASSED ]", " %zu test%s.", pass_count, Plural(pass_count)); + if (!failures.empty()) { + Print(kRed, "[ FAILED ]", " %zu test%s.", fail_count, Plural(fail_count)); + for (auto& failure : failures) { + Print(kRed, "[ FAILED ]", " %s", failure.c_str()); + } + } + return (fail_count == 0) ? 0 : 1; +} + +static void ShowHelp(bool full) { + fprintf(full ? stdout : stderr, "usage: %s [-v] FILE...\n", g_progname); + if (!full) exit(EXIT_FAILURE); + + printf( + "\n" + "Run tests.\n" + "\n" + "-v\tVerbose (show workings)\n"); + exit(EXIT_SUCCESS); +} + +int main(int argc, char* argv[]) { + g_progname = basename(argv[0]); + + static const struct option opts[] = { + {"help", no_argument, 0, 'h'}, + {"verbose", no_argument, 0, 'v'}, + {}, + }; + + int opt; + while ((opt = getopt_long(argc, argv, "hv", opts, nullptr)) != -1) { + switch (opt) { + case 'h': + ShowHelp(true); + break; + case 'v': + g_verbose = true; + break; + default: + ShowHelp(false); + break; + } + } + + argv += optind; + if (!*argv) Die(0, "no test files provided"); + std::vector tests; + for (; *argv; ++argv) CollectTests(&tests, *argv); + return RunTests(tests); +} diff --git a/libziparchive/Android.bp b/libziparchive/Android.bp index e3bb2abe1..a20ae035b 100644 --- a/libziparchive/Android.bp +++ b/libziparchive/Android.bp @@ -198,3 +198,15 @@ cc_fuzz { host_supported: true, corpus: ["testdata/*"], } + +sh_test { + name: "ziptool-tests", + src: "run-ziptool-tests-on-android.sh", + filename: "run-ziptool-tests-on-android.sh", + test_suites: ["general-tests"], + host_supported: true, + device_supported: false, + test_config: "ziptool-tests.xml", + data: ["cli-tests/**/*"], + target_required: ["cli-test", "ziptool"], +} diff --git a/libziparchive/cli-tests/files/example.zip b/libziparchive/cli-tests/files/example.zip new file mode 100644 index 0000000000000000000000000000000000000000..c3292e929d466733eb5b9106047a53900cfad380 GIT binary patch literal 1082 zcmaLWF-yZh6bJB2lUPjz1zYH#A_zhiES9VeB4V{mhAPBCY!VR>L8L-D3Jwl_0w={q z5Cp%2gM+)1tB5#AH#h$;NiWCcYQk&U;eLI0?;p3R8y2I>UVGk*zoZ{Jn`GfqZL2lH z^pC3JQJqCL(~Rnj#*E7e#$3L*jK#&tGJ% zMs*zoRmS+7*`9YiDv9xZ!P$V*n_c}jU{GaFz?~f!RFV?}fdCyzhcd?ZwSRol>2mBE z5S4_{&Pw?IVfF+l_<3vC_(d&c9Yq|OT5von33<4$n0I@ntCo1+FH&O`Z!A1KO@`J0 zMI~`=Q!7mCgJxc#{&0=6&lP&c2{4lc`~&6ZpFkRjoAc!a%F+N(kp!``d=fJxs09T?674aC_t4Vp@9sFiP+^XN7K*%R0Kxs gGM|%vmo(u=4koFAr%a4Fg!5rV+;zO&J(?=}1CW}sMF0Q* literal 0 HcmV?d00001 diff --git a/libziparchive/cli-tests/unzip.test b/libziparchive/cli-tests/unzip.test new file mode 100755 index 000000000..6e5cbf213 --- /dev/null +++ b/libziparchive/cli-tests/unzip.test @@ -0,0 +1,148 @@ +# unzip tests. + +# Note: since "master key", Android uses libziparchive for all zip file +# handling, and that scans the whole central directory immediately. Not only +# lookups by name but also iteration is implemented using the resulting hash +# table, meaning that any test that makes assumptions about iteration order +# will fail on Android. + +name: unzip -l +command: unzip -l $FILES/example.zip d1/d2/x.txt +after: [ ! -f d1/d2/x.txt ] +expected-stdout: + Archive: $FILES/example.zip + Length Date Time Name + --------- ---------- ----- ---- + 1024 2017-06-04 08:45 d1/d2/x.txt + --------- ------- + 1024 1 file +--- + +name: unzip -lq +command: unzip -lq $FILES/example.zip d1/d2/x.txt +after: [ ! -f d1/d2/x.txt ] +expected-stdout: + Length Date Time Name + --------- ---------- ----- ---- + 1024 2017-06-04 08:45 d1/d2/x.txt + --------- ------- + 1024 1 file +--- + +name: unzip -lv +command: unzip -lv $FILES/example.zip d1/d2/x.txt +after: [ ! -f d1/d2/file ] +expected-stdout: + Archive: $FILES/example.zip + Length Method Size Cmpr Date Time CRC-32 Name + -------- ------ ------- ---- ---------- ----- -------- ---- + 1024 Defl:N 11 99% 2017-06-04 08:45 48d7f063 d1/d2/x.txt + -------- ------- --- ------- + 1024 11 99% 1 file +--- + +name: unzip -v +command: unzip -v $FILES/example.zip d1/d2/x.txt +after: [ ! -f d1/d2/file ] +expected-stdout: + Archive: $FILES/example.zip + Length Method Size Cmpr Date Time CRC-32 Name + -------- ------ ------- ---- ---------- ----- -------- ---- + 1024 Defl:N 11 99% 2017-06-04 08:45 48d7f063 d1/d2/x.txt + -------- ------- --- ------- + 1024 11 99% 1 file +--- + +name: unzip one file +command: unzip -q $FILES/example.zip d1/d2/a.txt && cat d1/d2/a.txt +after: [ ! -f d1/d2/b.txt ] +expected-stdout: + a +--- + +name: unzip all files +command: unzip -q $FILES/example.zip +after: [ -f d1/d2/a.txt ] +after: [ -f d1/d2/b.txt ] +after: [ -f d1/d2/c.txt ] +after: [ -f d1/d2/empty.txt ] +after: [ -f d1/d2/x.txt ] +after: [ -d d1/d2/dir ] +expected-stdout: +--- + +name: unzip -o +before: mkdir -p d1/d2 +before: echo b > d1/d2/a.txt +command: unzip -q -o $FILES/example.zip d1/d2/a.txt && cat d1/d2/a.txt +expected-stdout: + a +--- + +name: unzip -n +before: mkdir -p d1/d2 +before: echo b > d1/d2/a.txt +command: unzip -q -n $FILES/example.zip d1/d2/a.txt && cat d1/d2/a.txt +expected-stdout: + b +--- + +# The reference implementation will create *one* level of missing directories, +# so this succeeds. +name: unzip -d shallow non-existent +command: unzip -q -d will-be-created $FILES/example.zip d1/d2/a.txt +after: [ -d will-be-created ] +after: [ -f will-be-created/d1/d2/a.txt ] +--- + +# The reference implementation will *only* create one level of missing +# directories, so this fails. +name: unzip -d deep non-existent +command: unzip -q -d oh-no/will-not-be-created $FILES/example.zip d1/d2/a.txt 2> stderr ; echo $? > status +after: [ ! -d oh-no ] +after: [ ! -d oh-no/will-not-be-created ] +after: [ ! -f oh-no/will-not-be-created/d1/d2/a.txt ] +after: grep -q "oh-no/will-not-be-created" stderr +after: grep -q "No such file or directory" stderr +# The reference implementation has *lots* of non-zero exit values, but we stick to 0 and 1. +after: [ $(cat status) -gt 0 ] +--- + +name: unzip -d exists +before: mkdir dir +command: unzip -q -d dir $FILES/example.zip d1/d2/a.txt && cat dir/d1/d2/a.txt +after: [ ! -f d1/d2/a.txt ] +expected-stdout: + a +--- + +name: unzip -p +command: unzip -p $FILES/example.zip d1/d2/a.txt +after: [ ! -f d1/d2/a.txt ] +expected-stdout: + a +--- + +name: unzip -x FILE... +# Note: the RI ignores -x DIR for some reason, but it's not obvious we should. +command: unzip -q $FILES/example.zip -x d1/d2/a.txt d1/d2/b.txt d1/d2/empty.txt d1/d2/x.txt && cat d1/d2/c.txt +after: [ ! -f d1/d2/a.txt ] +after: [ ! -f d1/d2/b.txt ] +after: [ ! -f d1/d2/empty.txt ] +after: [ ! -f d1/d2/x.txt ] +after: [ -d d1/d2/dir ] +expected-stdout: + ccc +--- + +name: unzip FILE -x FILE... +command: unzip -q $FILES/example.zip d1/d2/a.txt d1/d2/b.txt -x d1/d2/a.txt && cat d1/d2/b.txt +after: [ ! -f d1/d2/a.txt ] +after: [ -f d1/d2/b.txt ] +after: [ ! -f d1/d2/c.txt ] +after: [ ! -f d1/d2/empty.txt ] +after: [ ! -f d1/d2/x.txt ] +after: [ ! -d d1/d2/dir ] +expected-stdout: + bb +--- diff --git a/libziparchive/cli-tests/zipinfo.test b/libziparchive/cli-tests/zipinfo.test new file mode 100755 index 000000000..d5bce1c01 --- /dev/null +++ b/libziparchive/cli-tests/zipinfo.test @@ -0,0 +1,53 @@ +# zipinfo tests. + +# Note: since "master key", Android uses libziparchive for all zip file +# handling, and that scans the whole central directory immediately. Not only +# lookups by name but also iteration is implemented using the resulting hash +# table, meaning that any test that makes assumptions about iteration order +# will fail on Android. + +name: zipinfo -1 +command: zipinfo -1 $FILES/example.zip | sort +expected-stdout: + d1/ + d1/d2/a.txt + d1/d2/b.txt + d1/d2/c.txt + d1/d2/dir/ + d1/d2/empty.txt + d1/d2/x.txt +--- + +name: zipinfo header +command: zipinfo $FILES/example.zip | head -2 +expected-stdout: + Archive: $FILES/example.zip + Zip file size: 1082 bytes, number of entries: 7 +--- + +name: zipinfo footer +command: zipinfo $FILES/example.zip | tail -1 +expected-stdout: + 7 files, 1033 bytes uncompressed, 20 bytes compressed: 98.1% +--- + +name: zipinfo directory +# The RI doesn't use ISO dates. +command: zipinfo $FILES/example.zip d1/ | sed s/17-Jun-/2017-06-/ +expected-stdout: + drwxr-x--- 3.0 unx 0 bx stor 2017-06-04 08:40 d1/ +--- + +name: zipinfo stored +# The RI doesn't use ISO dates. +command: zipinfo $FILES/example.zip d1/d2/empty.txt | sed s/17-Jun-/2017-06-/ +expected-stdout: + -rw-r----- 3.0 unx 0 bx stor 2017-06-04 08:43 d1/d2/empty.txt +--- + +name: zipinfo deflated +# The RI doesn't use ISO dates. +command: zipinfo $FILES/example.zip d1/d2/x.txt | sed s/17-Jun-/2017-06-/ +expected-stdout: + -rw-r----- 3.0 unx 1024 tx defN 2017-06-04 08:45 d1/d2/x.txt +--- diff --git a/libziparchive/run-ziptool-tests-on-android.sh b/libziparchive/run-ziptool-tests-on-android.sh new file mode 100755 index 000000000..3c23d4373 --- /dev/null +++ b/libziparchive/run-ziptool-tests-on-android.sh @@ -0,0 +1,15 @@ +#!/bin/bash + +# Copy the tests across. +adb shell rm -rf /data/local/tmp/ziptool-tests/ +adb shell mkdir /data/local/tmp/ziptool-tests/ +adb push cli-tests/ /data/local/tmp/ziptool-tests/ +#adb push cli-test /data/local/tmp/ziptool-tests/ + +if tty -s; then + dash_t="-t" +else + dash_t="" +fi + +exec adb shell $dash_t cli-test /data/local/tmp/ziptool-tests/cli-tests/*.test diff --git a/libziparchive/unzip.cpp b/libziparchive/unzip.cpp index 11b575eae..dd42e90b9 100644 --- a/libziparchive/unzip.cpp +++ b/libziparchive/unzip.cpp @@ -52,7 +52,7 @@ enum Role { static Role role; static OverwriteMode overwrite_mode = kPrompt; static bool flag_1 = false; -static const char* flag_d = nullptr; +static std::string flag_d; static bool flag_l = false; static bool flag_p = false; static bool flag_q = false; @@ -214,12 +214,9 @@ static void ExtractOne(ZipArchiveHandle zah, ZipEntry& entry, const std::string& } // Where are we actually extracting to (for human-readable output)? - std::string dst; - if (flag_d) { - dst = flag_d; - if (!EndsWith(dst, "/")) dst += '/'; - } - dst += name; + // flag_d is the empty string if -d wasn't used, or has a trailing '/' + // otherwise. + std::string dst = flag_d + name; // Ensure the directory hierarchy exists. if (!MakeDirectoryHierarchy(android::base::Dirname(name))) { @@ -463,6 +460,7 @@ int main(int argc, char* argv[]) { switch (opt) { case 'd': flag_d = optarg; + if (!EndsWith(flag_d, "/")) flag_d += '/'; break; case 'l': flag_l = true; @@ -511,9 +509,17 @@ int main(int argc, char* argv[]) { } // Implement -d by changing into that directory. - // We'll create implicit directories based on paths in the zip file, but we - // require that the -d directory already exists. - if (flag_d && chdir(flag_d) == -1) die(errno, "couldn't chdir to %s", flag_d); + // We'll create implicit directories based on paths in the zip file, and we'll create + // the -d directory itself, but we require that *parents* of the -d directory already exists. + // This is pretty arbitrary, but it's the behavior of the original unzip. + if (!flag_d.empty()) { + if (mkdir(flag_d.c_str(), 0777) == -1 && errno != EEXIST) { + die(errno, "couldn't created %s", flag_d.c_str()); + } + if (chdir(flag_d.c_str()) == -1) { + die(errno, "couldn't chdir to %s", flag_d.c_str()); + } + } ProcessAll(zah); diff --git a/libziparchive/ziptool-tests.xml b/libziparchive/ziptool-tests.xml new file mode 100644 index 000000000..211119fe3 --- /dev/null +++ b/libziparchive/ziptool-tests.xml @@ -0,0 +1,26 @@ + + + +