{{ message }}
Repository navigation
Release v0.26.8 - #4865
Merged
Merged
Release v0.26.8#4865
Conversation
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 c2c95ad)
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 0661990)
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 7c8d460)
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 f031e20)
(cherry picked from commit 4da3819)
(cherry picked from commit 697583e)
(cherry picked from commit c582fa4)
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 0b98a66)
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 bf15dbf)
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 37bb151)
(cherry picked from commit 71ba464)
Move back to Travis's VM infrastructure for efficiency. (cherry picked from commit 9dc21ef)
The trusty dependencies are now hosted on Bintray. (cherry picked from commit da9898a)
The VM on Travis apparently will still proceed, but it's good practice. (cherry picked from commit 6e74813)
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 c17c3f8)
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 8999f6a)
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 72c28ab)
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 9aba763)
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 b8c1449)
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 543ec14)
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 fea6092)
Right now, we test our credential callback code twice, once via SSH on localhost and once via a non-existent GitHub repository. While the first URL makes sense to be configurable, it does not make sense to hard-code the non-existing repository, which requires us to call tests multiple times. Instead, we can just inline the URL into another set of tests. (cherry picked from commit 54a1bf0)
Some tests shall be run against our own SSH server we spin up in Travis. As those need to be run separate from our previous tests which run against git-daemon, we have to do this in a separate step. Instead of bundling all that knowledge in the CI script, move it into the test build instructions by creating a new test target. (cherry picked from commit 5874e15)
AppVeyor currently does provide three standard build worker images with VS2013, VS2015 and VS2017. Right now, we are using the implicitly, which is the VS2015 one. We want to be more explicit about this, so that we can easily switch build images based on the job. So starting from this commit, we explicitly set the `APPVEYOR_BUILD_WORKER_IMAGE` variable per job, which enables us to choose different images. To be able to test a wider range of build configurations, this commit also switches the jobs for VC2010 over to use the older, VS2013 based images. As the next commit will introduce two new jobs for building with VS2015, we have then covered both build environments. Also, let us be a bit more explicit regarding the CMake generator. Instead of only saying "Visual Studio 10", use the more descriptive value "Visual Studio 10 2010" to at least avoid some confusion surrounding the versioning scheme of Visual Studio. (cherry picked from commit e1076db)
In order to cover a wider range of build environments, add two more jobs which build and test libgit2 on Visual Studio 14 2015. (cherry picked from commit 03a95bc)
Our CI builds have intermittent failures in our online tests, e.g. with the message "A provided buffer was too small". This is not a programming error in libgit2 but rather an error in the SChannel component of Windows. Under certain circumstances involving Diffie-Hellman key exchange, SChannel is unable to correctly handle input from the server. This bug has already been fixed in recent patches for Windows 10 and Windows Server 2016, but they are not yet available for AppVeyor. Manually pamper over that issue by disabling all ciphersuites using DHE via the registry. While this disables more ciphers than necessary, we really don't care for that at all but just want to avoid build failures due to that bug. See [1], [2] or [3] for additional information. 1: aws/aws-sdk-cpp#671 2: https://github.com/dotnet/corefx/issues/7812 3: https://support.microsoft.com/en-us/help/2992611/ms14-066-vulnerability-in-schannel-could-allow-remote-code-execution-n (cherry picked from commit 723e1e9)
Commit 723e1e9 (appveyor: disable DHE to avoid spurious failures, 2018-03-29) added a workaround to fix spurious test failures due to a bug in Windows' SChannel implementation. The workaround only worked by accident, though, as the registry key was in fact mistyped. Fix the typo. (cherry picked from commit 3a72b0e)
When executing `libgit2_clar -smerge -invalid_option`, it will first execute the merge test suite and afterwards output help because of the invalid option. With this changa, it verifies all options before execute. If there are any invalid options, it will output help and exit without actually executing the test suites. (cherry picked from commit 3275863)
(cherry picked from commit 2f4e7cb)
(cherry picked from commit 74b0a43)
Similar to the way we parse the ctest output on POSIX systems, do the same on Windows. This allows us to append the `-r` flag to clar after we've identified the command to run. (cherry picked from commit 7c9769d)
Don't stop on test failures; run all the tests, even when a test fails. (cherry picked from commit 429c7f1)
PowerShell can _read_ top-level variables in functions, but cannot _update_ top-level variables in functions unless they're explicitly prefixed with `$global`. (cherry picked from commit 0e26717)
pks-t
force-pushed
the
pks/v0.26.8
branch
3 times, most recently
from
October 26, 2018 12:22
6f0b000 to
87a046f
Compare
(cherry picked from commit 820fb71)
Free the url field when resetting the stream to avoid leaking it. (cherry picked from commit ca2eb46)
Don't just free the push status structure, actually free the strings that were strdup'd into the struct as well. (cherry picked from commit dad9988)
Don't just free the spec vector, also free the specs themselves. (cherry picked from commit d285de7)
Before resetting the url and username, ensure that we free them in case they were set by environment variables. (cherry picked from commit e84914f)
When quick-parsing a commit, we use `git__strtol64` to parse the commit's time. The buffer that's passed to `commit_quick_parse` is the raw data of an ODB object, though, whose data may not be properly formatted and also does not have to be `NUL` terminated. This may lead to out-of-bound reads. Use `git__strntol64` to avoid this problem. (cherry picked from commit 1a3fa1f)
We use `git__strtol64` to parse file modes of the index entries, which does not limit the parsed buffer length. As the index can be essentially treated as "untrusted" in that the data stems from the file system, it may be misformatted and may not contain terminating `NUL` bytes. This may lead to out-of-bounds reads when trying to parse index entries with such malformatted modes. Fix the issue by using `git__strntol64` instead. (cherry picked from commit 600cead)
We use `git__strtol64` and `git__strtol32` to parse the trailing commit or author date and timezone of signatures. As signatures are usually part of a commit or tag object and thus essentially untrusted data, the buffer may be misformatted and may not be `NUL` terminated. This may lead to an out-of-bounds read. Fix the issue by using `git__strntol64` and `git__strntol32` instead. (cherry picked from commit 3db9aa6)
When parsing integers from configuration values, we use `git__strtol64`. This is fine to do, as we always sanitize values and can thus be sure that they'll have a terminating `NUL` byte. But as this is the last call-site of `git__strtol64`, let's just pass in the length explicitly by calling `strlen` on the value to be able to remove `git__strtol64` altogether. (cherry picked from commit 1a2efd1)
The function `git__strtol64` does not take a maximum buffer length as parameter. This has led to some unsafe usages of this function, and as such we may consider it as being unsafe to use. As we have now eradicated all usages of this function, let's remove it completely to avoid future misuse. (cherry picked from commit 68deb2c)
We use the `git__strtol32` function to parse the child and entry count of treecaches from the index, which do not accept a buffer length. As the buffer that is being passed in is untrusted data and may thus be malformed and may not contain a terminating `NUL` byte, we can overrun the buffer and thus perform an out-of-bounds read. Fix the issue by uzing `git__strntol32` instead. (cherry picked from commit 21652ee)
Replace remaining uses of the `git__strtol32` function. While these uses are all safe as the strings were either sanitized or from a trusted source, we want to remove `git__strtol32` altogether to avoid future misuse. (cherry picked from commit 2613fbb)
The function `git__strtol32` can easily be misused when untrusted data is passed to it that may not have been sanitized with trailing `NUL` bytes. As all usages of this function have now been removed, we can remove this function altogether to avoid future misuse of it. (cherry picked from commit 8d7fa88)
Some edge cases were currently completely untested, e.g. parsing numbers
greater than INT64_{MIN,MAX}, truncating buffers by length and invalid
characters. Add tests to verify that the system under test performs as
expected.
(cherry picked from commit 39087ab)
While `git__strntol64` tries to detect integer overflows when doing the necessary arithmetics to come up with the final result, it does the detection only after the fact. This check thus relies on undefined behavior of signed integer overflows. Fix this by instead checking up-front whether the multiplications or additions will overflow. Note that a detected overflow will not cause us to abort parsing the current sequence of digits. In the case of an overflow, previous behavior was to still set up the end pointer correctly to point to the first character immediately after the currently parsed number. We do not want to change this now as code may rely on the end pointer being set up correctly even if the parsed number is too big to be represented as 64 bit integer. (cherry picked from commit b09c1c7)
When an integer that is parsed with `git__strntol32` is too big to fit into an int32, we will generate an error message that includes the actual string that failed to parse. This does not acknowledge the fact that the string may either not be NUL terminated or alternative include additional characters after the number that is to be parsed. We may thus end up printing characters into the buffer that aren't the number or, worse, read out of bounds. Fix the issue by utilizing the `endptr` that was set by `git__strntol64`. This pointer is guaranteed to be set to the first character following the number, and we can thus use it to compute the width of the number that shall be printed. Create a test to verify that we correctly truncate the number. (cherry picked from commit ea19efc)
Unfortunately, neither the `memmem` nor the `strnstr` functions are part of any C standard but are merely extensions of C that are implemented by e.g. glibc. Thus, there is no standardized way to search for a string in a block of memory with a limited size, and using `strstr` is to be considered unsafe in case where the buffer has not been sanitized. In fact, there are some uses of `strstr` in exactly that unsafe way in our codebase. Provide a new function `git__memmem` that implements the `memmem` semantics. That is in a given haystack of `n` bytes, search for the occurrence of a byte sequence of `m` bytes and return a pointer to the first occurrence. The implementation chosen is the "Not So Naive" algorithm from [1]. It was chosen as the implementation is comparably simple while still being reasonably efficient in most cases. Preprocessing happens in constant time and space, searching has a time complexity of O(n*m) with a slightly sub-linear average case. [1]: http://www-igm.univ-mlv.fr/~lecroq/string/ (cherry picked from commit 83e8a6b)
When parsing tags, we skip all unknown fields that appear before the tag message. This skipping is done by using a plain `strstr(buffer, "\n\n")` to search for the two newlines that separate tag fields from tag message. As it is not possible to supply a buffer length to `strstr`, this call may skip over the buffer's end and thus result in an out of bounds read. As `strstr` may return a pointer that is out of bounds, the following computation of `buffer_end - buffer` will overflow and result in an allocation of an invalid length. Fix the issue by using `git__memmem` instead. Add a test that verifies parsing the tag fails not due to the allocation failure but due to the tag having no message. (cherry picked from commit ee11d47)
The commit message encoding is currently being parsed by the `git__prefixcmp` function. As this function does not accept a buffer length, it will happily skip over a buffer's end if it is not `NUL` terminated. Fix the issue by using `git__prefixncmp` instead. Add a test that verifies that we are unable to parse the encoding field if it's cut off by the supplied buffer length. (cherry picked from commit 7655b2d)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

No description provided.