On Mon, Dec 6, 2021 at 9:11 AM Christian MAUDERER <christian.maude...@embedded-brains.de> wrote: > > 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?
I think it is worth fixing. Go ahead. If there isn't any documentation on the untar family, please file a ticket on that. FWIW Ryan is trying to get all the tools to build on 5 so we can add it to the automated build sweep. There will likely be more patches like his dtc upgrade to allow things to build. --joel > > 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.maude...@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@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@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@K8~^|S > >> > >> -- > >> 2.31.1 > >> > >> _______________________________________________ > >> devel mailing list > >> devel@rtems.org > >> http://lists.rtems.org/mailman/listinfo/devel > > -- > -------------------------------------------- > embedded brains GmbH > Herr Christian MAUDERER > Dornierstr. 4 > 82178 Puchheim > Germany > email: christian.maude...@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/ _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel