[rtems commit] untar: Make behavior similar to GNU or BSD tar

Chris Johns chrisj at rtems.org
Thu Dec 9 07:33:59 UTC 2021


Hi Christian,

I did not know this was for the 5 branch. Where was this discussed?

Functional changes to a release branch need to be treated carefully and in this
case I would not approve it. I am sorry if I did not pick up it was for 5 as well.

Thanks
Chris

On 9/12/21 6:21 pm, Christian Mauderer wrote:
> Module:    rtems
> Branch:    5
> Commit:    ff3f3490df7120c9ec039b5acd1935265c3f9262
> Changeset: http://git.rtems.org/rtems/commit/?id=ff3f3490df7120c9ec039b5acd1935265c3f9262
> 
> Author:    Christian Mauderer <christian.mauderer at embedded-brains.de>
> Date:      Wed Dec  1 16:39:46 2021 +0100
> 
> untar: Make behavior similar to GNU or BSD tar
> 
> RTEMS untar implementation had problems with overwriting or integrating
> archives into existing directory structures. This patch adapts the
> behavior to mimic that of a GNU tar or BSD tar and extends the tar01
> test to check for the behavior. That is:
> 
> * If a directory structure exists, the files from the archive will be
>   integrated. Existing files are overwritten.
> 
> * If a file exists and the archive contains a directory with the same
>   name, the file is removed and a directory is created. In the above
>   example: if l1/l2 is a file it will be overwritten with a new
>   directory.
> 
> * If a directory exists and the archive contains a file with the same
>   name, the directory will be replaced if it is empty. If it contains
>   files, the result is an error.
> 
> * An archive also can contain only a file without the parent
>   directories. If in that case one of the parent directories exists as a
>   file extracting the archive results in an error. In the example: if
>   l1/l2 is a file and the archive doesn't contain the directories but
>   only the file l1/l2/x.txt that would be an error.
> 
> * In case of an error, it is possible that the archive has been
>   partially extracted.
> 
> Closes #4552
> 
> ---
> 
>  cpukit/libmisc/untar/untar.c        |  57 +++++++----
>  testsuites/libtests/tar01/init.c    | 199 +++++++++++++++++++++++++++++++++++-
>  testsuites/libtests/tar01/tar01.doc |   1 +
>  testsuites/libtests/tar01/tar01.scn |  54 ++++++++--
>  testsuites/libtests/tar01/tar01.tar | Bin 10240 -> 10240 bytes
>  5 files changed, 282 insertions(+), 29 deletions(-)
> 
> diff --git a/cpukit/libmisc/untar/untar.c b/cpukit/libmisc/untar/untar.c
> index a2f09fb..8888ab2 100644
> --- a/cpukit/libmisc/untar/untar.c
> +++ b/cpukit/libmisc/untar/untar.c
> @@ -126,30 +126,25 @@ Make_Path(const rtems_printer *printer, char *path)
>  
>      *p = '\0';
>      if (p[1] == '\0') {
> -      /* Speculatively unlink the last component so that it can be re-created */
> -      unlink(path);
>        return 0;
>      }
>  
>      if (mkdir(path, S_IRWXU | S_IRWXG | S_IRWXO) != 0) {
> -      if (errno == EEXIST || errno == EISDIR) {
> +      if (errno == EEXIST) {
> +        /* If it exists already: Check whether it is a directory */
>          struct stat sb;
> -
> -        if (stat(path, &sb) != 0) {
> +        if (lstat(path, &sb) != 0) {
> +          Print_Error(printer, "lstat", path);
> +          return -1;
> +        } else if (!S_ISDIR(sb.st_mode)) {
> +          rtems_printf(printer,
> +                       "untar: mkdir: %s: exists but is not a directory\n",
> +                       path);
>            return -1;
>          }
> -
> -        if (!S_ISDIR(sb.st_mode)) {
> -          if (unlink(path) != 0) {
> -            Print_Error(printer, "unlink", path);
> -            return -1;
> -          }
> -
> -          if (mkdir(path, S_IRWXU | S_IRWXG | S_IRWXO) != 0) {
> -            Print_Error(printer, "mkdir (unlink)", path);
> -            return -1;
> -          }
> -        }
> +      } else {
> +        Print_Error(printer, "mkdir", path);
> +        return -1;
>        }
>      }
>  
> @@ -206,6 +201,12 @@ Untar_ProcessHeader(
>  
>    if (Make_Path(ctx->printer, ctx->file_path) != 0) {
>      retval = UNTAR_FAIL;
> +  } else {
> +    /*
> +     * Speculatively unlink. This should unlink everything but non-empty
> +     * directories or write protected stuff.
> +     */
> +    unlink(ctx->file_path);
>    }
>  
>    if (ctx->linkflag == SYMTYPE) {
> @@ -225,8 +226,22 @@ Untar_ProcessHeader(
>      rtems_printf(ctx->printer, "untar: dir: %s\n", ctx->file_path);
>      r = mkdir(ctx->file_path, ctx->mode);
>      if (r != 0) {
> -      Print_Error(ctx->printer, "mkdir", ctx->file_path);
> -      retval = UNTAR_FAIL;
> +      if (errno == EEXIST) {
> +        /* If it exists already: Check whether it is a directory */
> +        struct stat sb;
> +        if (lstat(ctx->file_path, &sb) != 0) {
> +          Print_Error(ctx->printer, "lstat", ctx->file_path);
> +          retval = UNTAR_FAIL;
> +        } else if (!S_ISDIR(sb.st_mode)) {
> +          rtems_printf(ctx->printer,
> +                       "untar: mkdir: %s: exists but is not a directory\n",
> +                       ctx->file_path);
> +          retval = UNTAR_FAIL;
> +        }
> +      } else {
> +        Print_Error(ctx->printer, "mkdir", ctx->file_path);
> +        retval = UNTAR_FAIL;
> +      }
>      }
>    }
>  
> @@ -426,7 +441,9 @@ Untar_FromFile_Print(
>         */
>  
>        if ((out_fd = creat(ctx.file_path, ctx.mode)) == -1) {
> -        (void) lseek(fd, SEEK_CUR, 512UL * ctx.nblocks);
> +        /* Couldn't create that file. Abort. */
> +        retval = UNTAR_FAIL;
> +        break;
>        } else {
>          for (i = 0; i < ctx.nblocks; i++) {
>            n = read(fd, bufr, 512);
> diff --git a/testsuites/libtests/tar01/init.c b/testsuites/libtests/tar01/init.c
> index 4cad67a..2deff3a 100644
> --- a/testsuites/libtests/tar01/init.c
> +++ b/testsuites/libtests/tar01/init.c
> @@ -7,6 +7,33 @@
>   *  http://www.rtems.org/license/LICENSE.
>   */
>  
> +/*
> + * Note on the used tar file: Generate the file on a system that supports
> + * symlinks with the following commands (tested on Linux - you might have to
> + * adapt on other systems):
> + *
> + * export WORK=some_work_directory
> + * rm -r ${WORK}
> + * mkdir -p ${WORK}/home/abc/def
> + * mkdir -p ${WORK}/home/dir
> + * cd ${WORK}
> + * echo "#! joel" > home/abc/def/test_script
> + * echo "ls -las /dev" >> home/abc/def/test_script
> + * chmod 755 home/abc/def/test_script
> + * echo "This is a test of loading an RTEMS filesystem from an" > home/test_file
> + * echo "initial tar image." >> home/test_file
> + * echo "Hello world" >> home/dir/file
> + * ln -s home/test_file symlink
> + * tar cf tar01.tar --format=ustar \
> + *     symlink \
> + *     home/test_file \
> + *     home/abc/def/test_script \
> + *     home/dir
> + *
> + * Note that "home/dir" is in the archive as separate directory. "home/abc" is
> + * only in the archive as a parent of the file "test_script".
> + */
> +
>  #ifdef HAVE_CONFIG_H
>  #include "config.h"
>  #endif
> @@ -95,6 +122,84 @@ void test_untar_from_memory(void)
>  
>  }
>  
> +static void assert_file_content(
> +    const char *name,
> +    const char *expected_content,
> +    ssize_t expected_size
> +)
> +{
> +  char buf[16];
> +  int fd;
> +  int rd;
> +
> +  fd = open(name, O_RDONLY);
> +  rtems_test_assert( fd >= 0 );
> +  do {
> +    rd = read(fd, buf, sizeof(buf));
> +    rtems_test_assert( rd >= 0 );
> +    if (rd > 0) {
> +      rtems_test_assert( expected_size - rd >= 0 );
> +      rtems_test_assert( memcmp(buf, expected_content, rd) == 0 );
> +      expected_content += rd;
> +      expected_size -= rd;
> +    }
> +  } while(rd > 0);
> +  rtems_test_assert( expected_size == 0 );
> +  close(fd);
> +}
> +
> +static void assert_content_like_expected(void)
> +{
> +  const char *directories[] = {
> +    "home",
> +    "home/abc",
> +    "home/abc/def",
> +    "home/dir",
> +  };
> +  const char *symlinks[] = {
> +    "symlink",
> +  };
> +  const struct {
> +    const char *name;
> +    const char *content;
> +  } files[] = {
> +    {
> +      .name = "home/abc/def/test_script",
> +      .content = "#! joel\nls -las /dev\n",
> +    }, {
> +      .name = "home/test_file",
> +      .content = "This is a test of loading an RTEMS filesystem from an\n"
> +                 "initial tar image.\n",
> +    }, {
> +      .name = "home/dir/file",
> +      .content = "Hello world\n",
> +    }
> +  };
> +  size_t i;
> +  struct stat st;
> +
> +  for(i = 0; i < RTEMS_ARRAY_SIZE(directories); ++i) {
> +    lstat(directories[i], &st);
> +    rtems_test_assert( S_ISDIR(st.st_mode) );
> +  }
> +
> +  for(i = 0; i < RTEMS_ARRAY_SIZE(symlinks); ++i) {
> +    lstat(symlinks[i], &st);
> +    rtems_test_assert( S_ISLNK(st.st_mode) );
> +  }
> +
> +  for(i = 0; i < RTEMS_ARRAY_SIZE(files); ++i) {
> +    lstat(files[i].name, &st);
> +    rtems_test_assert( S_ISREG(st.st_mode) );
> +
> +    assert_file_content(
> +        files[i].name,
> +        files[i].content,
> +        strlen(files[i].content)
> +        );
> +  }
> +}
> +
>  void test_untar_from_file(void)
>  {
>    int                fd;
> @@ -119,13 +224,105 @@ void test_untar_from_file(void)
>    rv = chdir( "/dest" );
>    rtems_test_assert( rv == 0 );
>  
> -  /* Untar it */
> +  /* Case 1: Untar it into empty directory */
>    rv = Untar_FromFile( "/test.tar" );
>    printf("Untaring from file - ");
>    if (rv != UNTAR_SUCCESSFUL) {
>      printf ("error: untar failed: %i\n", rv);
>      exit(1);
>    }
> +  assert_content_like_expected();
> +  printf ("successful\n");
> +
> +  /* Case 2: Most files exist */
> +  rv = unlink("/dest/home/test_file");
> +  rtems_test_assert( rv == 0 );
> +
> +  rv = Untar_FromFile( "/test.tar" );
> +  printf("Untar from file into existing structure with one missing file - ");
> +  if (rv != UNTAR_SUCCESSFUL) {
> +    printf ("error: untar failed: %i\n", rv);
> +    exit(1);
> +  }
> +  assert_content_like_expected();
> +  printf ("successful\n");
> +
> +  /* Case 3: An empty directory exists where a file should be */
> +  rv = unlink("/dest/home/test_file");
> +  rtems_test_assert( rv == 0 );
> +  rv = mkdir("/dest/home/test_file", S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
> +  rtems_test_assert( rv == 0 );
> +
> +  rv = Untar_FromFile( "/test.tar" );
> +  printf("Untar from file; overwrite empty directory with file - ");
> +  if (rv != UNTAR_SUCCESSFUL) {
> +    printf ("error: untar failed: %i\n", rv);
> +    exit(1);
> +  }
> +  assert_content_like_expected();
> +  printf ("successful\n");
> +
> +  /* Case 4: A file exists where a parent directory should be created */
> +  rv = unlink("/dest/home/abc/def/test_script");
> +  rtems_test_assert( rv == 0 );
> +  rv = unlink("/dest/home/abc/def");
> +  rtems_test_assert( rv == 0 );
> +  rv = unlink("/dest/home/abc");
> +  rtems_test_assert( rv == 0 );
> +  fd = creat("/dest/home/abc", S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> +  rtems_test_assert( fd >= 0 );
> +  close(fd);
> +
> +  rv = Untar_FromFile( "/test.tar" );
> +  printf("Untar from file; file exists where parent dir should be created - ");
> +  if (rv != UNTAR_FAIL) {
> +    printf ("error: untar didn't fail like expected: %i\n", rv);
> +    exit(1);
> +  }
> +  printf ("expected fail\n");
> +  /* cleanup so that the next one works */
> +  rv = unlink("/dest/home/abc");
> +  rtems_test_assert( rv == 0 );
> +
> +  /* Case 5: A non-empty directory exists where a file should be created */
> +  rv = unlink("/dest/home/test_file");
> +  rtems_test_assert( rv == 0 );
> +  rv = mkdir("/dest/home/test_file", S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
> +  rtems_test_assert( rv == 0 );
> +  fd = creat("/dest/home/test_file/file",
> +      S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> +  rtems_test_assert( fd >= 0 );
> +  close(fd);
> +
> +  rv = Untar_FromFile( "/test.tar" );
> +  printf("Untar from file; non-empty dir where file should be created - ");
> +  if (rv != UNTAR_FAIL) {
> +    printf ("error: untar didn't fail like expected: %i\n", rv);
> +    exit(1);
> +  }
> +  printf ("expected fail\n");
> +  /* cleanup so that the next one works */
> +  rv = unlink("/dest/home/test_file/file");
> +  rtems_test_assert( rv == 0 );
> +  rv = unlink("/dest/home/test_file");
> +  rtems_test_assert( rv == 0 );
> +
> +  /* Case 6: A file exists where a directory is explicitly in the archive */
> +  rv = unlink("/dest/home/dir/file");
> +  rtems_test_assert( rv == 0 );
> +  rv = unlink("/dest/home/dir");
> +  rtems_test_assert( rv == 0 );
> +  fd = creat("/dest/home/dir", S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> +  rtems_test_assert( fd >= 0 );
> +  close(fd);
> +
> +  rv = Untar_FromFile( "/test.tar" );
> +  printf("Untar from file; overwrite file with explicit directory - ");
> +  if (rv != UNTAR_SUCCESSFUL) {
> +    printf ("error: untar failed: %i\n", rv);
> +    exit(1);
> +  }
> +  assert_content_like_expected();
>    printf ("successful\n");
>  
>    /******************/
> diff --git a/testsuites/libtests/tar01/tar01.doc b/testsuites/libtests/tar01/tar01.doc
> index 060f98a..adffdca 100644
> --- a/testsuites/libtests/tar01/tar01.doc
> +++ b/testsuites/libtests/tar01/tar01.doc
> @@ -20,3 +20,4 @@ directives:
>  concepts:
>  
>  + exercise these routines
> ++ check whether existing files are overwritten or not overwritten like expected
> diff --git a/testsuites/libtests/tar01/tar01.scn b/testsuites/libtests/tar01/tar01.scn
> index 68fa951..dd72f95 100644
> --- a/testsuites/libtests/tar01/tar01.scn
> +++ b/testsuites/libtests/tar01/tar01.scn
> @@ -1,9 +1,24 @@
> -*** TAR01 TEST ***
> -Untaring from memory - successful
> +*** BEGIN OF TEST TAR 1 ***
> +*** TEST VERSION: 6.0.0.e1efb4eb8a9d6dd5f6f37dafc9feb0a9e6a888f1
> +*** TEST STATE: EXPECTED_PASS
> +*** TEST BUILD: RTEMS_POSIX_API
> +*** TEST TOOLS: 10.3.1 20210409 (RTEMS 6, RSB ad54d1dd3cf8249d9d39deb1dd28b2f294df062d-modified, Newlib eb03ac1)
> +Untaring from memory - untar: memory at 0x11ece8 (10240)
> +untar: symlink: home/test_file -> symlink
> +untar: file: home/test_file (s:73,m:0644)
> +untar: file: home/abc/def/test_script (s:21,m:0755)
> +untar: dir: home/dir
> +untar: file: home/dir/file (s:12,m:0644)
> +successful
>  ========= /home/test_file =========
>  (0)This is a test of loading an RTEMS filesystem from an
>  initial tar image.
>  
> +========= /home/abc/def/test_script =========
> +(0)#! joel
> +ls -las /dev
> +
> + /home/abc/def/test_script: mode: 0755 want: 0755
>  ========= /symlink =========
>  (0)This is a test of loading an RTEMS filesystem from an
>  initial tar image.
> @@ -11,35 +26,58 @@ initial tar image.
>  
>  Copy tar image to test.tar
>  Untaring from file - successful
> +Untar from file into existing structure with one missing file - successful
> +Untar from file; overwrite empty directory with file - successful
> +Untar from file; file exists where parent dir should be created - expected fail
> +Untar from file; non-empty dir where file should be created - expected fail
> +Untar from file; overwrite file with explicit directory - successful
>  ========= /dest/home/test_file =========
>  (0)This is a test of loading an RTEMS filesystem from an
>  initial tar image.
>  
> +========= /dest/home/abc/def/test_script =========
> +(0)#! joel
> +ls -las /dev
> +
> + /dest/home/abc/def/test_script: mode: 0755 want: 0755
>  ========= /dest/symlink =========
>  (0)This is a test of loading an RTEMS filesystem from an
>  initial tar image.
>  
>  
> -Untaring chunks from memory - untar: dir: home
> -untar: file: home/test_file (73)
> +Untaring chunks from memory - untar: symlink: home/test_file -> symlink
> +untar: file: home/test_file (s:73,m:0644)
> +untar: file: home/abc/def/test_script (s:21,m:0755)
> +untar: dir: home/dir
> +untar: file: home/dir/file (s:12,m:0644)
>  successful
>  ========= /dest2/home/test_file =========
>  (0)This is a test of loading an RTEMS filesystem from an
>  initial tar image.
>  
> +========= /dest2/home/abc/def/test_script =========
> +(0)#! joel
> +ls -las /dev
> +
> + /dest2/home/abc/def/test_script: mode: 0755 want: 0755
>  ========= /dest2/symlink =========
>  (0)This is a test of loading an RTEMS filesystem from an
>  initial tar image.
>  
>  
> -Untaring chunks from tgz- untar: dir: home
> -untar: file: home/test_file (73)
> -successful
> +Untaring chunks from tgz - successful
>  ========= /dest3/home/test_file =========
>  (0)This is a test of loading an RTEMS filesystem from an
>  initial tar image.
>  
> +========= /dest3/home/abc/def/test_script =========
> +(0)#! joel
> +ls -las /dev
> +
> + /dest3/home/abc/def/test_script: mode: 0755 want: 0755
>  ========= /dest3/symlink =========
>  (0)This is a test of loading an RTEMS filesystem from an
>  initial tar image.
> -*** END OF TAR01 TEST ***
> +
> +
> +*** END OF TEST TAR 1 ***
> diff --git a/testsuites/libtests/tar01/tar01.tar b/testsuites/libtests/tar01/tar01.tar
> index 6c6952e..9874f42 100644
> Binary files a/testsuites/libtests/tar01/tar01.tar and b/testsuites/libtests/tar01/tar01.tar differ
> 
> _______________________________________________
> vc mailing list
> vc at rtems.org
> http://lists.rtems.org/mailman/listinfo/vc
> 


More information about the devel mailing list