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 ignored facilities #9

Merged
merged 2 commits into from
Sep 18, 2015
Merged

Fix ignored facilities #9

merged 2 commits into from
Sep 18, 2015

Conversation

rmg
Copy link
Member

@rmg rmg commented Sep 18, 2015

I believe this to be the root of #8. The problem isn't specific to FreeBSD, but the fact that Haraka was using syslog.open() to specify a log facility by number (syslog.LOG_MAIL instead of 'LOG_MAIL').

It's a pretty trivial change, so we should be able to publish a patch release shortly.

@sam-github PTAL
/cc @Dexus @msimerson

Connect to #8

Ensure that something silly doesn't happen, like toFacility(0) doing
a reverse lookup and returning 'LOG_KERN' instead of a number.
Fix a bug in toFacility that resulted in toFacility(0) returning the
string 'LOG_KERN' (a reverse lookup). Because toFacility is used
by syslog.open() this was resulting in all numeric (aka, pre-resolved)
log facilities being mapped to a string which always converted to 0.

This should fix the problem reported in #8 and first discovered in
baudehlo/Haraka#1145
@msimerson
Copy link
Contributor

👍 thanks greatly for your effort in tracking that down.

@msimerson
Copy link
Contributor

I just tested that patch against my production Haraka and it does indeed solve the problem.

@Dexus
Copy link

Dexus commented Sep 18, 2015

👍 now it also runtastic 💯 on docker image.

Thanks Ryan. Can only agree with Matt.

@sam-github
Copy link
Contributor

LGTM, can you publish after merge?

rmg added a commit that referenced this pull request Sep 18, 2015
@rmg rmg merged commit eef3eb3 into master Sep 18, 2015
@rmg rmg removed the #review label Sep 18, 2015
@rmg rmg deleted the fix-ignored-facilities branch September 18, 2015 21:45
@rmg
Copy link
Member Author

rmg commented Sep 18, 2015

Released [email protected]

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.

4 participants