Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pkg/stanza] Fix issue where syslog octet parsing could truncate token #27294

Merged

Conversation

djaglowski
Copy link
Member

This fixes a bug detected while attempting to migrate test to the new splittest framework.

Generally speaking, the responsibility of a bufio.SplitFunc is to parse a token from a given buffer ([]byte). However, the split func does not have control over the size of the buffer, so it must be able to ask for more data. The mechanism for asking for more data is to return 0, nil, nil.

A split func is also told whether there is any more data to read. This allows it to chose whether to "give up" and return a truncated token, or to insist that it will wait until there is more data (which may never happen).

This particular function is parsing tokens based on a simple numerical prefix which indicates how long the token will be.
e.g. 54 This is the actual token and it is 54 characters long.

The problem is that the function would give up prematurely and return a truncated token. The proper behavior is to ask for more data unless the function is specifically told that there is no more data to receive.

This fixes the behavior so that whenever we are able to parse an expected length but find there is not enough data in the buffer to fulfill the expectation, we ask for more data. It only returns a truncated token when there is no more data to ask for.

@djaglowski djaglowski force-pushed the pkg-stanza-syslog-octen-split branch from 64d6524 to f2a7b7b Compare October 2, 2023 17:19
@djaglowski djaglowski marked this pull request as ready for review October 2, 2023 17:45
@djaglowski djaglowski requested review from a team and TylerHelmuth October 2, 2023 17:45
@djaglowski djaglowski merged commit d9aba7b into open-telemetry:main Oct 2, 2023
93 checks passed
@djaglowski djaglowski deleted the pkg-stanza-syslog-octen-split branch October 2, 2023 19:43
@github-actions github-actions bot added this to the next release milestone Oct 2, 2023
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
open-telemetry#27294)

This fixes a bug detected while attempting to migrate test to the new
`splittest` framework.

Generally speaking, the responsibility of a `bufio.SplitFunc` is to
parse a token from a given buffer (`[]byte`). However, the split func
does not have control over the size of the buffer, so it must be able to
ask for more data. The mechanism for asking for more data is to return
`0, nil, nil`.

A split func is also told whether there is any more data to read. This
allows it to chose whether to "give up" and return a truncated token, or
to insist that it will wait until there is more data (which may never
happen).

This particular function is parsing tokens based on a simple numerical
prefix which indicates how long the token will be.
e.g. `54 This is the actual token and it is 54 characters long.`

The problem is that the function would give up prematurely and return a
truncated token. The proper behavior is to ask for more data _unless_
the function is specifically told that there is no more data to receive.

This fixes the behavior so that whenever we are able to parse an
expected length but find there is not enough data in the buffer to
fulfill the expectation, we ask for more data. It only returns a
truncated token when there is no more data to ask for.
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