#50440 Issue 49232 - Truncate the message when buffer capacity is exceeded
Closed by spichugi. Opened by spichugi.
spichugi/389-ds-base fix-access-buf-exceeded  into  master

Download 50440.patch

Bug Description: When the access log buffer capacity is exceeded we log
an emergency error and the access log line is not logged at all.

Fix Description: Log the error message to errors log and log the access
log message but truncate its elements (for the search access log message).
Or just log what is in the buffer in other cases.

https://pagure.io/389-ds-base/issue/49232

Reviewed by: ?

rebased onto 0ccc1b3c445a3e00a7b7f01267bbaf9f4ed05bde

I've added a basic CI test for the feature.

Also, I did run ldclt searches on two similar setups (Vagrant machines with and without PR) and the performance isn't affected (at least, I haven't seen any noticeable difference in the results)

I've decided not to perform the same smart cutting on modify, modrdn and add access log messages. Instead, if something malicious will happen - it will be logged to the errors log and the access log message will be truncated by the buffer size (2048 - time part).

typo "abd" should be "and"

Looks good, but can you test if it works correctly then the "attribute list", filter, and base dn all exceed the buffer? Where they are all truncated?

Looks good, but can you test if it works correctly then the "attribute list", filter, and base dn all exceed the buffer? Where they are all truncated?

I've tested it manually and it truncates at 512 characters each.
I'll extend the CI test then (just need to figure out if we have attribute list functionality in DSLdapObject...)

1 new commit added

  • Improve CI test

Typo is fixed and the test case is improved.

Can you do a ASAN build to make sure there are no issues? If good, then ACK!

Yeah, I see nothing obviously wrong here either :)

The patch looks great. Just a minor comment. If message was longer than buffer size, it used to return -1 and skip syslog/journal logging. If we truncate the message should not be careful and continue to skip syslog/journal logs ?

Journal and syslog don't have the same limits as our logs do they? I think they write immediately, they don't buffer the content.

1 new commit added

  • Return -1 if overflow error was encountered

I think it makes sense to return -1 if the access log message was truncated and ERR was reported to errors log.

@tbordaz please, check.

Also, I was able to run tests with ASAN (Thanks, @vashirov !) and it doesn't have any leaks related to my change.

The patch is looking good to me. Ack

rebased onto d4a676cff41fb8840d1ae9ceb9e3d6c9faf6b389

Pull-Request has been merged by spichugi

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3498

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

Metadata