I can explain my reasoning for opening the PR in the first place. I figure there are three different ways to proceed:
- Make no change, except for a comment in the config file noting that passwords containing # won’t be parsed properly
- Change the config parser to allow hashes in SMTP passwords without quotes - possibly by requiring whitespace before the # for it to be interpreted as a comment.
- Add quotes around the values in the generated discourse.conf file.
I ended up going with option 3 since it seemed the least of 3 evils.
I didn’t like option 1 because I think it’s important for security reasons to allow as many different characters as possible. In addition, the SMTP password I was using was given to me by our ops guy, and he was reluctant to change it for other business reasons.
Option 2 seemed the most difficult to implement correctly, and I was also worried about usability issues with having a commenting system that operated differently from a standard system. I was worried that someone would add a comment to the end of the SMTP password and then get confused when the comment was interpreted as part of the password.
Option 3 isn’t great either, for the reasons that @sam and @codinghorror have mentioned. But if every value in the .conf file is wrapped in quotes like my PR does, I think that someone looking at the generated .conf file would notice that pattern.
Anyway, we’ve got to do something, at least. It took me a long time to figure out why my password wasn’t working, and clearly I’m not the only one.
Unfortunately I don’t have access to the system I was using for development and testing, so I can’t contribute any more code.