openenclave / oeedger8r-cpp

An implementation of oeedger8r in C++
MIT License
8 stars 13 forks source link

Fix for buffer overflow scenario when input_buffer_size is smaller th… #58

Closed mrragava closed 4 years ago

mrragava commented 4 years ago

…an argument

Signed-off-by: mrragava mrragava@gmail.com

oe-ci-robot commented 4 years ago

Welcome @mrragava! It looks like this is your first PR to openenclave/oeedger8r-cpp 🎉

oe-ci-robot commented 4 years ago

Hi @mrragava. Thanks for your PR.

I'm waiting for a openenclave member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
mingweishih commented 4 years ago

/lgtm

mingweishih commented 4 years ago

/approve

BRMcLaren commented 4 years ago

/ok-to-test

oe-ci-robot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anakrish, mingweishih, mrragava

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/openenclave/oeedger8r-cpp/blob/master/OWNERS)~~ [anakrish,mingweishih] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
BRMcLaren commented 4 years ago

/retest

mrragava commented 4 years ago

/retest

anakrish commented 4 years ago

@BRMcLaren Is this good to be merged? The status says "Some checks haven't completed yet"

BRMcLaren commented 4 years ago

@anakrish Looking into right now, thank you for bringing this to my attention. Seems everything passed, maybe an issue with the k8's infra?

To answer your question, yes this is good to merge and we can manually merge if needed. This can be determined by 1) it passed all prow tests and 2) it passed all the legacy backend tests (which are the same tests) on all platforms.

BRMcLaren commented 4 years ago

@anakrish Looked into it, there was a broken pipe issue in the infra. It was rectified by restarting the container, will keep an eye out for resurgencee.

Unfortunately.. the prow jobs are blocked by https://github.com/openenclave-ci/test-infra/pull/237. As they are the same as the legacy infra (continuous-integration/jenkins/pr-head continuous-integration/jenkins/pr-merge) and we know those work I am gonna override the jobs as it does not diminish our test coverage.

Sorry for the delay @mrragava ! We are in the middle of a CI overhaul, thanks for your contribution.

/override pr-oeedger8r-cpp-rhel8-clang-8-debug /override pr-oeedger8r-cpp-rhel8-clang-8-reldebinfo /override pr-oeedger8r-cpp-rhel8-clang-8-release /override pr-oeedger8r-cpp-windows-2016-debug /override pr-oeedger8r-cpp-windows-2016-release /override pr-oeedger8r-cpp-windows-2019-debug /override pr-oeedger8r-cpp-windows-2019-release

oe-ci-robot commented 4 years ago

@BRMcLaren: Overrode contexts on behalf of BRMcLaren: pr-oeedger8r-cpp-rhel8-clang-8-debug, pr-oeedger8r-cpp-rhel8-clang-8-reldebinfo, pr-oeedger8r-cpp-rhel8-clang-8-release, pr-oeedger8r-cpp-windows-2016-debug, pr-oeedger8r-cpp-windows-2016-release, pr-oeedger8r-cpp-windows-2019-debug, pr-oeedger8r-cpp-windows-2019-release

In response to [this](https://github.com/openenclave/oeedger8r-cpp/pull/58#issuecomment-720595426): >@anakrish Looked into it, there was a broken pipe issue in the infra. It was rectified by restarting the container, will keep an eye out for resurgencee. > >Unfortunately.. the prow jobs are blocked by https://github.com/openenclave-ci/test-infra/pull/237. As they are the same as the legacy infra (continuous-integration/jenkins/pr-head continuous-integration/jenkins/pr-merge) and we know those work I am gonna override the jobs as it does not diminish our test coverage. > >Sorry for the delay @mrragava ! We are in the middle of a CI overhaul, thanks for your contribution. > >/override pr-oeedger8r-cpp-rhel8-clang-8-debug >/override pr-oeedger8r-cpp-rhel8-clang-8-reldebinfo >/override pr-oeedger8r-cpp-rhel8-clang-8-release >/override pr-oeedger8r-cpp-windows-2016-debug >/override pr-oeedger8r-cpp-windows-2016-release >/override pr-oeedger8r-cpp-windows-2019-debug >/override pr-oeedger8r-cpp-windows-2019-release > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.