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

Allow comment characters within configuration values when using my.cnf #769

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

buzztaiki
Copy link

This PR will fix #376

I've configured SpaceBeforeInlineComment option to allow comment characters within values.
see https://github.com/go-ini/ini/blob/b2f570e5b5b844226bbefe6fb521d891f529a951/ini.go#L97-L101

@SuperQ
Copy link
Member

SuperQ commented Sep 12, 2023

What about passwords with # or ; in them?

@SuperQ
Copy link
Member

SuperQ commented Sep 12, 2023

Oh, I see, this is to allow this. Wouldn't quotes used be a safer way to handle this?

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

What prevents quotes around the value from solving this problem?

@patrickeasters
Copy link

patrickeasters commented Sep 18, 2023

EDIT: It turns out you can escape comment chars by using backticks or triple quotes.

I stumbled upon this PR because I hit the same issue. Even when quoted, comment characters are still interpreted as the beginning of comments.

Here's a quick playspace demonstrating that using the same ini package: https://goplay.space/#dAkouM7WJ1T

@SuperQ
Copy link
Member

SuperQ commented Sep 18, 2023

@patrickeasters Thanks for digging into the quoting. Perhaps we should update the README docs to recommend backtick quoting of passwords.

@SuperQ
Copy link
Member

SuperQ commented Sep 18, 2023

I would also be fine with simply flipping IgnoreInlineComment: true.

@patrickeasters
Copy link

Either of those sound better than the rabbit hole of reading docs of mysqld_exporter's dependencies and cursing the inconsistency of INI parsers. 😄

Updating the README is of course the safest, but I don't imagine a ton of folks using inline comments in these INI files either.

@SuperQ
Copy link
Member

SuperQ commented Sep 19, 2023

This needs a DCO sign-off. You can use git commit -s --amend to add it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mysql passwords with # and ;
3 participants