Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

I'm going to accept, to remove it from my review queue, but I think @fmanco should weigh in on this (or @akindyakov)

Thanks for working on improving osquery tests @nasehim7 . If you could rebase it on the experimental branch, I will merge it soon.

@nasehim7 Seems reasonable. https://dev.azure.com/trailofbits/osquery/_build/results?buildId=651 says formatting error

@directionless Thanks! I am not able to understand where exactly I am messing it up. I am not using an IDE instead just a simple editor(Sublime Text)

@nasehim7 The linter is running tools/formatting/format-check.py. AFAIK at the root of it, it's using clang-format.

I also don't use an IDE. I manually run my code through clang-format -i to fix the formatting.

@nasehim7 The linter is running tools/formatting/format-check.py. AFAIK at the root of it, it's using clang-format.

I also don't use an IDE. I manually run my code through clang-format -i to fix the formatting.

It's not the same as calling clang-format on the whole file though, the script checks and eventually format only the modified lines, which is what we would like to maintain since it creates less noise and conflicts between different versions of clang-format used.

@Smjert Understandable, thanks for the explanation. It seems the formatting is fixed now but it is throwing:

/Users/vsts/agent/2.155.1/work/1/s/tests/integration/tables/curl_certificate.cpp:37:30: error: expected '}'
{ {"hostname", NormalType} {"common_name", NormalType} {
^
/Users/vsts/agent/2.155.1/work/1/s/tests/integration/tables/curl_certificate.cpp:37:3: note: to match this '{'
{ {"hostname", NormalType} {"common_name", NormalType} {
^
1 error generated.

Not sure why is it throwing that because if there was some parenthesis mismatch, it wouldn't have done the building with buck. Is it because of some vital differences in the build system?

@Smjert Understandable, thanks for the explanation. It seems the formatting is fixed now but it is throwing:

/Users/vsts/agent/2.155.1/work/1/s/tests/integration/tables/curl_certificate.cpp:37:30: error: expected '}'
{ {"hostname", NormalType} {"common_name", NormalType} {
^
/Users/vsts/agent/2.155.1/work/1/s/tests/integration/tables/curl_certificate.cpp:37:3: note: to match this '{'
{ {"hostname", NormalType} {"common_name", NormalType} {
^
1 error generated.

Not sure why is it throwing that because if there was some parenthesis mismatch, it wouldn't have done the building with buck. Is it because of some vital differences in the build system?

The error is not showing with Buck because it's not running those tests, good catch!
I'll land a PR to add them soon.

Hi @nasehim7, sorry for leaving this pending, are you still interested on working on this PR?

The curl_certificates table got some updates, especially there are many more columns now.
Also the openssl library has been updated, so the issues you were facing with the table not returning columns should've been fixed; at least, I just tried and it works.

So you would need to:

1. Rebase this on latest master
2. Fix the build, adding the rest of the columns, plus changing the name of the map type from ValidatatioMap to ValidationMap
3. Format the code before pushing, from the build folder calling make format or cmake --build . --target format.

Heads up @nasehim7, I rebased this onto our master.

This PR has proved helpful! The certificate_has_expired column was never being set because in the row structure it was set to has_expired.

I think we should change the column API here to remove the certificate_ prefixes on columns.