Repository: bazelbuild/bazel
Issue: zipper: File foo�ؖ� does not seem to exist.

### Description of the problem / feature request:

When invoking zipper with an argument file (`@args.txt`), it occasionally fails with an error of `File <yourfilename> <junk> does not seem to exist`. It looks like a string termination bug.

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

```console
$ touch foo bar
# Insert a newline between lines, but don't include a terminating \n.
$ echo foo > args.txt; echo -n bar >> args.txt
$ zipper Cc foo.zip @args.txt
```

### What operating system are you running Bazel on?

CentOS 7.2 and macOS High Sierra

### What's the output of `bazel info release`?

1.1.0+vmware

### If `bazel info release` returns "development version" or "(@non-git)", tell us how you built Bazel.

na

### What's the output of `git remote get-url origin ; git rev-parse master ; git rev-parse HEAD` ?

na

###  Have you found anything relevant by searching the web?

No hits.

### Any other information, logs, or outputs that you want to share?

Looks like a string termination bug in zip_main.cc:

https://github.com/bazelbuild/bazel/blob/8075057af6108ebc23c146f18eecec911d4b8c00/third_party/ijar/zip_main.cc#L253-L274

Each '\n' is replaced by '\0', but if a file doesn't end with '\n', the final file entry isn't terminated. Behavior is then undefined.

Valgrind also shows that something is awry:

```console
beasleyr@sc-dbc1213 tmp.qmQJvjLcyS$ valgrind .../zipper c foo.zip @args.txt
==1872== Memcheck, a memory error detector
==1872== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==1872== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==1872== Command: .../zipper c foo.zip @args.txt
==1872==
==1872== Conditional jump or move depends on uninitialised value(s)
==1872==    at 0x4A07B5A: index (mc_replace_strmem.c:222)
==1872==    by 0x403839: devtools_ijar::parse_filelist(char*, char**, int, bool) (in .../zipper)
==1872==    by 0x403907: devtools_ijar::create(char*, char**, bool, bool, bool) (in .../zipper)
==1872==    by 0x3B13A1ED5C: (below main) (libc-start.c:226)
==1872==
==1872== Conditional jump or move depends on uninitialised value(s)
==1872==    at 0x4A07B5E: index (mc_replace_strmem.c:222)
==1872==    by 0x403839: devtools_ijar::parse_filelist(char*, char**, int, bool) (in .../zipper)
==1872==    by 0x403907: devtools_ijar::create(char*, char**, bool, bool, bool) (in .../zipper)
==1872==    by 0x3B13A1ED5C: (below main) (libc-start.c:226)
==1872==
==1872== Syscall param stat(file_name) points to uninitialised byte(s)
==1872==    at 0x3B13ADADF5: _xstat (xstat.c:38)
==1872==    by 0x406F82: devtools_ijar::stat_file(char const*, devtools_ijar::Stat*) (in .../zipper)
==1872==    by 0x4063C8: devtools_ijar::ZipBuilder::EstimateSize(char const* const*, char const* const*, int) (in .../zipper)
==1872==    by 0x403929: devtools_ijar::create(char*, char**, bool, bool, bool) (in .../zipper)
==1872==    by 0x3B13A1ED5C: (below main) (libc-start.c:226)
==1872==  Address 0x4c33cef is 31 bytes inside a block of size 32 alloc'd
==1872==    at 0x4A06A2E: malloc (vg_replace_malloc.c:270)
==1872==    by 0x4036A9: devtools_ijar::read_filelist(char*) (in .../zipper)
==1872==    by 0x402803: main (in .../zipper)
==1872==
==1872== Conditional jump or move depends on uninitialised value(s)
==1872==    at 0x4A07FA8: strlen (mc_replace_strmem.c:403)
==1872==    by 0x4063E2: devtools_ijar::ZipBuilder::EstimateSize(char const* const*, char const* const*, int) (in .../zipper)
==1872==    by 0x403929: devtools_ijar::create(char*, char**, bool, bool, bool) (in .../zipper)
==1872==    by 0x3B13A1ED5C: (below main) (libc-start.c:226)
==1872==
==1872== Syscall param stat(file_name) points to uninitialised byte(s)
==1872==    at 0x3B13ADADF5: _xstat (xstat.c:38)
==1872==    by 0x406F82: devtools_ijar::stat_file(char const*, devtools_ijar::Stat*) (in .../zipper)
==1872==    by 0x4033CB: devtools_ijar::add_file(std::unique_ptr<devtools_ijar::ZipBuilder, std::default_delete<devtools_ijar::ZipBuilder> > const&, char*, char*, bool, bool, bool) (in .../zipper)
==1872==    by 0x403996: devtools_ijar::create(char*, char**, bool, bool, bool) (in .../zipper)
==1872==    by 0x3B13A1ED5C: (below main) (libc-start.c:226)
==1872==  Address 0x4c33cef is 31 bytes inside a block of size 32 alloc'd
==1872==    at 0x4A06A2E: malloc (vg_replace_malloc.c:270)
==1872==    by 0x4036A9: devtools_ijar::read_filelist(char*) (in .../zipper)
==1872==    by 0x402803: main (in .../zipper)
==1872==
==1872== Conditional jump or move depends on uninitialised value(s)
==1872==    at 0x4A07FA8: strlen (mc_replace_strmem.c:403)
==1872==    by 0x40342A: devtools_ijar::add_file(std::unique_ptr<devtools_ijar::ZipBuilder, std::default_delete<devtools_ijar::ZipBuilder> > const&, char*, char*, bool, bool, bool) (in .../zipper)
==1872==    by 0x403996: devtools_ijar::create(char*, char**, bool, bool, bool) (in .../zipper)
==1872==    by 0x3B13A1ED5C: (below main) (libc-start.c:226)
==1872==
==1872== Conditional jump or move depends on uninitialised value(s)
==1872==    at 0x4A081A5: strncpy (mc_replace_strmem.c:476)
==1872==    by 0x40345D: devtools_ijar::add_file(std::unique_ptr<devtools_ijar::ZipBuilder, std::default_delete<devtools_ijar::ZipBuilder> > const&, char*, char*, bool, bool, bool) (in .../zipper)
==1872==    by 0x403996: devtools_ijar::create(char*, char**, bool, bool, bool) (in .../zipper)
==1872==    by 0x3B13A1ED5C: (below main) (libc-start.c:226)
==1872==
==1872==
==1872== HEAP SUMMARY:
==1872==     in use at exit: 72,902 bytes in 7 blocks
==1872==   total heap usage: 13 allocs, 6 frees, 89,461 bytes allocated
==1872==
==1872== LEAK SUMMARY:
==1872==    definitely lost: 160 bytes in 3 blocks
==1872==    indirectly lost: 38 bytes in 3 blocks
==1872==      possibly lost: 0 bytes in 0 blocks
==1872==    still reachable: 72,704 bytes in 1 blocks
==1872==         suppressed: 0 bytes in 0 blocks
==1872== Rerun with --leak-check=full to see details of leaked memory
==1872==
==1872== For counts of detected and suppressed errors, rerun with: -v
==1872== Use --track-origins=yes to see where uninitialised values come from
==1872== ERROR SUMMARY: 7 errors from 7 contexts (suppressed: 8 from 6)
```

Issue comments:
Thank you for the fix!

On CentOS 7 this is what I get:

```
$ ~/bazel-master/bazel-bin/third_party/ijar/zipper Cc foo.zip @args.txt
File bart does not seem to exist.
```

Even though args.txt says `bar`, not `bart`. I guess that's the memory junk your PR fixes. Thanks!

Pull request review comments:

Patch that resolved the issue:
diff --git a/third_party/ijar/zip_main.cc b/third_party/ijar/zip_main.cc
index eac095e136c042..b375da39ecc236 100644
--- a/third_party/ijar/zip_main.cc
+++ b/third_party/ijar/zip_main.cc
@@ -251,11 +251,12 @@ char **read_filelist(char *filename) {
   }
 
   size_t sizeof_array = sizeof(char *) * (nb_entries + 1);
-  void *result = malloc(sizeof_array + file_stat.total_size);
+  void *result = malloc(sizeof_array + file_stat.total_size + 1);
   // copy the content
   char **filelist = static_cast<char **>(result);
   char *content = static_cast<char *>(result) + sizeof_array;
   memcpy(content, data, file_stat.total_size);
+  content[file_stat.total_size] = '\0';
   free(data);
   // Create the corresponding array
   int j = 1;


Test patch: