1. 19 Oct, 2018 21 commits
    • tests: online::clone: construct credential-URL from environment · a1a495f2
      We support two types of passing credentials to the proxy, either via the
      URL or explicitly by specifying user and password. We test these types
      by modifying the proxy URL and executing the tests twice, which is
      in fact unnecessary and requires us to maintain the list of environment
      variables and test executions across multiple CI infrastructures.
      
      To fix the situation, we can just always pass the host, port, user and
      password to the tests. The tests can then assemble the complete URL
      either with or without included credentials, allowing us to test both
      cases in-process.
      
      (cherry picked from commit fea60920)
      Patrick Steinhardt committed
    • tests: perf: build but exclude performance tests by default · 89641431
      Our performance tests (or to be more concrete, our single performance
      test) are not built by default, as they are always #ifdef'd out. While
      it is true that we don't want to run performance tests by default, not
      compiling them at all may cause code rot and is thus an unfavorable
      approach to handle this.
      
      We can easily improve this situation: this commit removes the #ifdef,
      causing the code to always be compiled. Furthermore, we add `-xperf` to
      the default command line parameters of `generate.py`, thus causing the
      tests to be excluded by default.
      
      Due to this approach, we are now able to execute the performance tests
      by passing `-sperf` to `libgit2_clar`. Unfortunately, we cannot execute
      the performance tests on Travis or AppVeyor as they rely on history
      being available for the libgit2 repository. As both do a shallow clone
      only, though, this is not given.
      
      (cherry picked from commit 543ec149)
      Patrick Steinhardt committed
    • tests: iterator::workdir: fix reference count in stale test · 98378a3f
      The test `iterator::workdir::filesystem_gunk` is usually not executed,
      as it is guarded by the environment variable "GITTEST_INVASIVE_SPEED"
      due to its effects on speed. As such, it has become stale and does not
      account for new references which have meanwhile been added to the
      testrepo, causing it to fail. Fix this by raising the number of expected
      references to 15.
      
      (cherry picked from commit b8c14499)
      Patrick Steinhardt committed
    • tests: iterator_helpers: assert number of iterator items · d2bbea82
      When the function `expect_iterator_items` surpasses the number of
      expected items, we simply break the loop. This causes us to trigger an
      assert later on which has message attached, which is annoying when
      trying to locate the root error cause. Instead, directly assert that the
      current count is still smaller or equal to the expected count inside of
      the loop.
      
      (cherry picked from commit 9aba7636)
      Patrick Steinhardt committed
    • tests: status::worktree: indicate skipped tests on Win32 · 293c5ef2
      Some function bodies of tests which are not applicable to the Win32
      platform are completely #ifdef'd out instead of calling `cl_skip()`.
      This leaves us with no indication that these tests are not being
      executed at all and may thus cause decreased scrutiny when investigating
      skipped tests. Improve the situation by calling `cl_skip()` instead of
      just doing nothing.
      
      (cherry picked from commit 72c28ab0)
      Patrick Steinhardt committed
    • travis: build sources with tracing enabled · 8ba43299
      Our tracing architecture is not built by default, causing the Travis CI
      to not execute some code and skip several tests. As AppVeyor has already
      enabled the tracing architecture when building the code, we should do
      the same for Travis CI to have this code being tested on macOS and
      Linux.
      
      Add "-DENABLE_TRACE=ON" to our release-build options of Travis.
      
      (cherry picked from commit 8999f6ac)
      Patrick Steinhardt committed
    • travis: drop support for Ubuntu Precise · 13a6b203
      Ubuntu Precise is end of life since April 2017. At that point in time,
      Precise was still the main distro on which Travis CI built upon, with
      the Trusty-based images still being in a beta state. But since June
      21st, Trusty has officially moved out of beta and is now the default
      image for all new builds. Right now, we build on both old and new images
      to assure we support both.
      
      Unfortunately, this leaves us with the highest minimum version for CMake
      being 2.8.7, as Precise has no greater version in its repositories. And
      because of this limitation, we cannot actually use object libraries in
      our build instructions. But considering Precise is end of life and
      Trusty is now the new default for Travis, we can and should drop support
      for this old and unmaintained distribution. And so we do.
      
      (cherry picked from commit c17c3f8a)
      Patrick Steinhardt committed
    • travis: we use bintray's own key for signing · 76ecd892
      The VM on Travis apparently will still proceed, but it's good practice.
      
      (cherry picked from commit 6e748130)
      Carlos Martín Nieto committed
    • travis: fetch trusty dependencies from bintray · 6be03667
      The trusty dependencies are now hosted on Bintray.
      
      (cherry picked from commit da9898ab)
      Edward Thomson committed
    • travis: add custom apt sources · 0c51ecf2
      Move back to Travis's VM infrastructure for efficiency.
      
      (cherry picked from commit 9dc21efd)
      Edward Thomson committed
    • travis: let's try a 5GB ramdisk · 93434828
      (cherry picked from commit 71ba4644)
      Carlos Martín Nieto committed
    • travis: put clar's sandbox in a ramdisk on macOS · 4eecbdd0
      The macOS tests are by far the slowest right now. This attempts to remedy the
      situation somewhat by asking clar to put its test data on a ramdisk.
      
      (cherry picked from commit 37bb1512)
      Carlos Martín Nieto committed
    • examples: network: fix Win32 linking errors due to getline · 736356a6
      The getline(3) function call is not part of ISO C and, most importantly,
      it is not implemented on Microsoft Windows platforms. As our networking
      example code makes use of getline, this breaks builds on MSVC and MinGW.
      As this code wasn't built prior to the previous commit, this was never
      noticed.
      
      Fix the error by instead implementing a `readline` function, which
      simply reads the password from stdin until it reads a newline
      character.
      
      (cherry picked from commit bf15dbf6)
      Patrick Steinhardt committed
    • appveyor: build examples · 1c85bcd8
      By default, CMake will not build our examples directory. As we do not
      instruct either the MinGW or MSVC builds on AppVeyor to enable building
      these examples, we cannot verify that those examples at least build on
      Windows systems.
      
      Fix that by passing `-DBUILD_EXAMPLES=ON` to AppVeyor's CMake
      invocation.
      
      (cherry picked from commit 0b98a66b)
      Patrick Steinhardt committed
    • travis: only install custom libcurl on trusty · dc413239
      (cherry picked from commit c582fa4e)
      Edward Thomson committed
    • travis: only kill our own sshd · 7d1c72a4
      (cherry picked from commit 697583ea)
      Edward Thomson committed
    • travis: use trusty · fad7f7a2
      (cherry picked from commit 4da38193)
      Edward Thomson committed
    • travis: build with patched libcurl · 16957a7f
      Ubuntu trusty has a bug in curl when using NTLM credentials in a proxy,
      dereferencing a null pointer and causing segmentation faults.  Use a
      custom-patched version of libcurl that avoids this issue.
      
      (cherry picked from commit f031e20b)
      Edward Thomson committed
    • travis: upgrade container to Ubuntu 14.04 · 5491d0e1
      Ubuntu 12.04 (Precise Pangolin) reaches end of life on April 28th, 2017.
      As such, we should update our build infrastructure to use the next
      available LTS release, which is Ubuntu 14.04 LTS (Trusty Tahr). Note
      that Trusty is still considered beta quality on Travis. But considering
      we are able to correctly build and test libgit2, this seems to be a
      non-issue for us.
      
      Switch over our default distribution to Trusty. As Precise still has
      extended support for paying customers, add an additional job which
      compiles libgit2 on the old release.
      
      (cherry picked from commit 7c8d460f)
      Patrick Steinhardt committed
    • travis: cibuild: set up our own sshd server · 76a7d5f1
      Some tests of ours require to be running against an SSH server.
      Currently, we simply run against the SSH server provided and started by
      Travis itself. As our Linux tests run in a sudo-less environment, we
      have no control over its configuration and startup/shutdown procedure.
      While this has been no problem until now, it will become a problem as
      soon as we migrate over to newer Precise images, as the SSH server does
      not have any host keys set up. Luckily, we can simply set up our own
      unpriviledged SSH server. This has the benefit of us being able to
      modify its configuration even in a sudo-less environment.
      
      This commit sets up the unpriviledged SSH server on port 2222.
      
      (cherry picked from commit 06619904)
      Patrick Steinhardt committed
    • tests: online::clone: use URL of test server · b988f544
      All our tests running against a local SSH server usually read the
      server's URL from environment variables. But online::clone::ssh_cert
      test fails to do so and instead always connects to
      "ssh://localhost/foo". This assumption breaks whenever the SSH server is
      not running on the standard port, e.g. when it is running as a user.
      
      Fix the issue by using the URL provided by the environment.
      
      (cherry picked from commit c2c95ad0)
      Patrick Steinhardt committed
  2. 05 Oct, 2018 7 commits
  3. 03 Oct, 2018 12 commits
    • smart_pkt: do not accept callers passing in no line length · 21a2318b
      Right now, we simply ignore the `linelen` parameter of
      `git_pkt_parse_line` in case the caller passed in zero. But in fact, we
      never want to assume anything about the provided buffer length and
      always want the caller to pass in the available number of bytes.
      And in fact, checking all the callers, one can see that the funciton is
      never being called in case where the buffer length is zero, and thus we
      are safe to remove this check.
      
      (cherry picked from commit 1bc5b05c)
      Patrick Steinhardt committed
    • smart_pkt: return parsed length via out-parameter · 5836d8b6
      The `parse_len` function currently directly returns the parsed length of
      a packet line or an error code in case there was an error. Instead,
      convert this to our usual style of using the return value as error code
      only and returning the actual value via an out-parameter. Thus, we can
      now convert the output parameter to an unsigned type, as the size of a
      packet cannot ever be negative.
      
      While at it, we also move the check whether the input buffer is long
      enough into `parse_len` itself. We don't really want to pass around
      potentially non-NUL-terminated buffers to functions without also passing
      along the length, as this is dangerous in the unlikely case where other
      callers for that function get added. Note that we need to make sure
      though to not mess with `GIT_EBUFS` error codes, as these indicate not
      an error to the caller but that he needs to fetch more data.
      
      (cherry picked from commit c05790a8)
      Patrick Steinhardt committed
    • smart_pkt: reorder and rename parameters of `git_pkt_parse_line` · 3bbda7d7
      The parameters of the `git_pkt_parse_line` function are quite confusing.
      First, there is no real indicator what the `out` parameter is actually
      all about, and it's not really clear what the `bufflen` parameter refers
      to. Reorder and rename the parameters to make this more obvious.
      
      (cherry picked from commit 0b3dfbf4)
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "unpack" packets · a8356af8
      When checking whether an "unpack" packet returned the "ok" status or
      not, we use a call to `git__prefixcmp`. In case where the passed line
      isn't properly NUL terminated, though, this may overrun the line buffer.
      Fix this by using `git__prefixncmp` instead.
      
      (cherry picked from commit 5fabaca8)
      Patrick Steinhardt committed
    • smart_pkt: fix "ng" parser accepting non-space character · 02e4b27f
      When parsing "ng" packets, we blindly assume that the character
      immediately following the "ng" prefix is a space and skip it. As the
      calling function doesn't make sure that this is the case, we can thus
      end up blindly accepting an invalid packet line.
      
      Fix the issue by using `git__prefixncmp`, checking whether the line
      starts with "ng ".
      
      (cherry picked from commit b5ba7af2)
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "ok" packets · 8cd0a897
      There are two different buffer overflows present when parsing "ok"
      packets. First, we never verify whether the line already ends after
      "ok", but directly go ahead and also try to skip the expected space
      after "ok". Second, we then go ahead and use `strchr` to scan for the
      terminating newline character. But in case where the line isn't
      terminated correctly, this can overflow the line buffer.
      
      Fix the issues by using `git__prefixncmp` to check for the "ok " prefix
      and only checking for a trailing '\n' instead of using `memchr`. This
      also fixes the issue of us always requiring a trailing '\n'.
      
      Reported by oss-fuzz, issue 9749:
      
      Crash Type: Heap-buffer-overflow READ {*}
      Crash Address: 0x6310000389c0
      Crash State:
        ok_pkt
        git_pkt_parse_line
        git_smart__store_refs
      
      Sanitizer: address (ASAN)
      (cherry picked from commit a9f1ca09)
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "ACK" packets · 82c3fc33
      We are being quite lenient when parsing "ACK" packets. First, we didn't
      correctly verify that we're not overrunning the provided buffer length,
      which we fix here by using `git__prefixncmp` instead of
      `git__prefixcmp`. Second, we do not verify that the actual contents make
      any sense at all, as we simply ignore errors when parsing the ACKs OID
      and any unknown status strings. This may result in a parsed packet
      structure with invalid contents, which is being silently passed to the
      caller. This is being fixed by performing proper input validation and
      checking of return codes.
      
      (cherry picked from commit bc349045)
      Patrick Steinhardt committed
    • smart_pkt: adjust style of "ref" packet parsing function · 3fd6ce0d
      While the function parsing ref packets doesn't have any immediately
      obvious buffer overflows, it's style is different to all the other
      parsing functions. Instead of checking buffer length while we go, it
      does a check up-front. This causes the code to seem a lot more magical
      than it really is due to some magic constants. Refactor the function to
      instead make use of the style of other packet parser and verify buffer
      lengths as we go.
      
      (cherry picked from commit 5edcf5d1)
      Patrick Steinhardt committed
    • smart_pkt: check whether error packets are prefixed with "ERR " · e14dab2f
      In the `git_pkt_parse_line` function, we determine what kind of packet
      a given packet line contains by simply checking for the prefix of that
      line. Except for "ERR" packets, we always only check for the immediate
      identifier without the trailing space (e.g. we check for an "ACK"
      prefix, not for "ACK "). But for "ERR" packets, we do in fact include
      the trailing space in our check. This is not really much of a problem at
      all, but it is inconsistent with all the other packet types and thus
      causes confusion when the `err_pkt` function just immediately skips the
      space without checking whether it overflows the line buffer.
      
      Adjust the check in `git_pkt_parse_line` to not include the trailing
      space and instead move it into `err_pkt` for consistency.
      
      (cherry picked from commit 786426ea)
      Patrick Steinhardt committed
    • smart_pkt: explicitly avoid integer overflows when parsing packets · cfb9802b
      When parsing data, progress or error packets, we need to copy the
      contents of the rest of the current packet line into the flex-array of
      the parsed packet. To keep track of this array's length, we then assign
      the remaining length of the packet line to the structure. We do have a
      mismatch of types here, as the structure's `len` field is a signed
      integer, while the length that we are assigning has type `size_t`.
      
      On nearly all platforms, this shouldn't pose any problems at all. The
      line length can at most be 16^4, as the line's length is being encoded
      by exactly four hex digits. But on a platforms with 16 bit integers,
      this assignment could cause an overflow. While such platforms will
      probably only exist in the embedded ecosystem, we still want to avoid
      this potential overflow. Thus, we now simply change the structure's
      `len` member to be of type `size_t` to avoid any integer promotion.
      
      (cherry picked from commit 40fd84cc)
      Patrick Steinhardt committed
    • smart_pkt: honor line length when determining packet type · a7e87dd5
      When we parse the packet type of an incoming packet line, we do not
      verify that we don't overflow the provided line buffer. Fix this by
      using `git__prefixncmp` instead and passing in `len`. As we have
      previously already verified that `len <= linelen`, we thus won't ever
      overflow the provided buffer length.
      
      (cherry picked from commit 4a5804c9)
      Patrick Steinhardt committed
    • tests: verify parsing logic for smart packets · 5d108c9a
      The commits following this commit are about to introduce quite a lot of
      refactoring and tightening of the smart packet parser. Unfortunately, we
      do not yet have any tests despite our online tests that verify that our
      parser does not regress upon changes. This is doubly unfortunate as our
      online tests aren't executed by default.
      
      Add new tests that exercise the smart parsing logic directly by
      executing `git_pkt_parse_line`.
      
      (cherry picked from commit 365d2720)
      Patrick Steinhardt committed