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

fix(database/influxdb): replace invisible chars and whitespaces with space #235

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

grische
Copy link
Contributor

@grische grische commented May 27, 2024

Description

Sanitize tag values before they are being sent to Influx.
This fixes issues where users use '\n' in tag values (e.g. the owner) that is incompatible with InfluxDB's Line protocol.'

Thanks a lot for @T0biii for the help in tracing this error and creating a solution!

Motivation and Context

We keep on seeing errors in the yanic log about

May 27 16:08:33 gw04.in.ffmuc.net yanic[4157917]: time="2024-05-27T16:08:33.759+02:00" level="error" msg="{\"error\":\"partial write: unable to parse 'node,...,owner=peter@hase': missing fields dropped=0\"}\n" caller="database.go:165 github.com/FreifunkBremen/yanic/database/influxdb.(*Connection).addWorker"

Because the user uses peter@hase\n.de as their owner name. This works in other parts of yanic, but causes issues with influx.

Checklist:

  • My code follows the code style of this project.
  • I have added also tests for my new code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@grische grische changed the title replace invisible chars and whitespaces with space fix: replace invisible chars and whitespaces with space May 27, 2024
Copy link
Member

@corny corny left a comment

Choose a reason for hiding this comment

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

Please move the new into a separate function.

// https://docs.influxdata.com/influxdb/v2/reference/syntax/line-protocol/
// Line protocol does not support the newline character \n in tag or field values.
// To be safe, remove all non-printable characters and spaces except ASCII space and U+0020.
for _, tag := range tags {
Copy link
Member

Choose a reason for hiding this comment

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

Please move this loop into a new function

Copy link

Choose a reason for hiding this comment

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

@corny thanks for the hint. @grische moved this in a new function.
We would be happy to get a new review :)

@grische grische force-pushed the fix/influxdb-linebreak-error branch from 4e60c98 to 8de9029 Compare May 28, 2024 12:11
Copy link
Member

@genofire genofire left a comment

Choose a reason for hiding this comment

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

looks good

database/influxdb/database.go Outdated Show resolved Hide resolved
This fixes issues where users use '\n' in tag values (e.g. the owner)
that is incompatible with InfluxDB's Line protocol.
@grische grische force-pushed the fix/influxdb-linebreak-error branch from 8de9029 to e87b23e Compare June 11, 2024 14:03
@grische grische requested a review from corny June 11, 2024 14:04
@genofire genofire changed the title fix: replace invisible chars and whitespaces with space fix(database/influxdb): replace invisible chars and whitespaces with space Jun 11, 2024
@genofire genofire merged commit ef3ee35 into FreifunkBremen:main Jun 11, 2024
2 checks passed
Copy link

🎉 This PR is included in version 1.5.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@grische grische deleted the fix/influxdb-linebreak-error branch June 11, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants