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

Christian MAUDERER christian.mauderer at embedded-brains.de
Mon Dec 6 15:11:44 UTC 2021


Hello Joel,

thanks for the review. I'll apply it on master in the next few days.

The behavior that our untar can't overwrite directories is in 5 too. The 
behavior on 5 changed last with the patches from ticket #3823 in 
2019-11. I would lean to creating a ticket for 5 and apply the patch 
there too. What do you think?

Best regards

Christian

Am 03.12.21 um 18:34 schrieb Joel Sherrill:
> I don't see anything that would stop this from being committed.
> 
> This does make me realize that I think all of the untar capabilities
> are undocumented.
> 
> --joel
> 
> On Fri, Dec 3, 2021 at 6:51 AM Christian Mauderer
> <christian.mauderer at embedded-brains.de> wrote:
>>
>> 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 a2f09fb99f..8888ab2c57 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 4cad67a6ae..2deff3a482 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 060f98a813..adffdca291 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 68fa951881..dd72f9517b 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 6c6952ef183d51a9635c6ebbc8ed0443e8da1725..9874f426d15eb817f270af7dab48fce1f36be848 100644
>> GIT binary patch
>> literal 10240
>> zcmeH}-%i6I6vlhKr#QxI9dtmSz{DGGboUFc)S*yJ=`?$L>C6^qR=rsp<Gzz7G(h<C
>> z`}hvTjd!+w$%YJ;q9}@tv3w?S_NDAvwvlncWzL0CN at Ofg9c4!ODtI$(O>7sHb!M0)
>> zu4Aj4w4~*#v9YzfUihOEmBDLUucK+=(e~X&cIDAdf3I>3#2pePlCC}abIuR-=Srnx
>> zLTn6NXRiNzWh13eQ|B`Z6}g~GTR)Mm>3)85_duQZ;wHAnQ`H2Y8YZ^3$tsSLb;x at C
>> zWUeP;XoWdWPGIYQ{kWW#rus~^aoO0{_LLKK{x78L^*_tyq5pFx(scp<pHhULu`Vy@
>> zIT$x_k*=JMl+M8$yxarpA69SuU)p9m;NE`qKVygf=Tb6}ksNSM`=QVL{@?3A+zUJ9
>> z{h!}|jEisjtC`@)|NApK;LojbF3@{uT#2+lAZH8>0w4eaAOHd&00JNY0w4eaAOHd&
>> V00JNY0w4eaAOHd&00LtX_yCHOyx{-<
>>
>> literal 10240
>> zcmeI!-)h1z6b5jw`xIyFdh?`zpJ11}*}Ru%Q=2VqM^7F2_Dfp^QXGoV;n)`hsW~ah
>> z&wOdirZUUUNVhJmGmBXo`<BKrF^5qc^3iwS!>p7d6{RW-lY}|){VG$LubX8ylbi0P
>> z)9r#;%tlLRb-gYp72{>zRNB_G7t_kAf^jMR;)X`2YHskiRcVF%M^nEEs42)bI=A(X
>> zbWJa-r{`DdH-;RfGZhtWQ~8d-*49%w@^oY?y)%!&4XzDSIxkT6`PZB6GB?F|4#>8(
>> zAKm$pmsj+G$dcqJ_uuEhm=z9_N&Z8{afbX~PJDIc{T;nE#syC3!D%7C4?dCQt2zJN
>> zKsi*-*6*{<d&qz0^Zdi}-}g`R&nYJq4u{jsFDL)}`9JF)kDEgP0uX=z1Rwwb2tWV=
>> g5P$##AOHafKmY;|fB*y_009U<00Izzz^@5>0Tj at K8~^|S
>>
>> --
>> 2.31.1
>>
>> _______________________________________________
>> devel mailing list
>> devel at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel

-- 
--------------------------------------------
embedded brains GmbH
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email: christian.mauderer at embedded-brains.de
phone: +49-89-18 94 741 - 18
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/


More information about the devel mailing list