Skip to content

Conversation

aisk
Copy link
Member

@aisk aisk commented Aug 16, 2025

Please note that the lpUserSid parameter in ReportEvent don't added because it needs passing a new struct from Python to it, and we don't need this parameter for the usage of implement NTEventLogHandler. If we want add it in the future, we can add it as kwargs.

Docuements for these functions:

Copy link
Contributor

@sharktide sharktide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the test fails :)

@aisk
Copy link
Member Author

aisk commented Aug 18, 2025

Sorry, I forgot to check the CI result after the push.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your PR @aisk. It look interesting.

Could you please show how can this be used in NTEventLogHandler? Maybe not in this PR, but in a separate PR created on top of this?

PyObject *item = PyList_GetItem(strings, i);
if (!PyUnicode_Check(item)) {
PyMem_Free(string_array);
PyErr_SetString(PyExc_TypeError, "All strings must be unicode");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"unicode" is a weird term. There is no longer type unicode. I would suggest something like: "expected a list of strings, not %T".

// Handle raw data
if (raw_data->buf != NULL) {
data = raw_data->buf;
data_size = (DWORD) raw_data->len;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible integer overflow.

@aisk aisk marked this pull request as draft September 8, 2025 15:52
@aisk aisk marked this pull request as ready for review September 9, 2025 17:27
@aisk
Copy link
Member Author

aisk commented Sep 9, 2025

Thank you for your PR @aisk. It look interesting.

Could you please show how can this be used in NTEventLogHandler? Maybe not in this PR, but in a separate PR created on top of this?

Thanks for the view! I'll create another PR based on this to try to implement the whole feature in the original issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants