qmail-inject: fix header parse regression #229
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DerDakon
requested review from
josuah,
leahneukirchen,
mbhangui and
schmonz
as code owners
DerDakon
force-pushed
the
Dakon-inject-fix
branch
from
e557386
to
a87da64
Compare
|
Includes the changes from #231 to make the CI green. |
DerDakon
force-pushed
the
Dakon-inject-fix
branch
2 times, most recently
from
6af938c
to
5faf063
Compare
|
Could you try rebasing onto #232? Guessing it'll be easy and then we can be really sure. |
|
schmonz-Dakon-inject-fix-with-qfilter-and-tests, a branch with this change rebased onto #232 (and therefore also #227), passes all our checks. We'd usually want to first merge the prerequisite-to-tests #227, then squash the tests from #232 onto this commit, then merge this. But #227 is sort-of-blocked on @bruceg issuing relicensed code. Having seen the tests pass is good enough for me for the moment, provided we don't forget to merge them as soon as we can. I approve this PR on its own, now. |
schmonz
approved these changes
Jan 28, 2022
When attempting to catch incorrectly formatted headers like "To: foo: bar, baz" to avoid errors when injecting bounces the host expansion of headers like "To: root" was also broken. Instead of completely ignoring these lines when the recipient list was already given on the command line using -a option try to process these lines as before, but allow the parsing of these lines to silently fail. If a broken line is encountered this now simply does not reformat them to insert the hostname. Fixes: 56e7c4a
DerDakon
force-pushed
the
Dakon-inject-fix
branch
from
5faf063
to
e7e9fc5
Compare
mbhangui
approved these changes
Feb 15, 2022
leahneukirchen
approved these changes
Feb 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When attempting to catch incorrectly formatted headers like "To: foo: bar, baz" to avoid errors when injecting bounces the host expansion of headers like "To: root" was also broken.
Instead of completely ignoring these lines when the recipient list was already given on the command line using -a option try to process these lines as before, but allow the parsing of these lines to silently fail. If a broken line is encountered this now simply does not reformat them to insert the hostname.
Bug introduced in commit 56e7c4a in 1.08.
Fixes: #147
As I already said in that bug: I think we should just merge this without the tests so we can get a 1.09 out of the door. This fixes the regression, and when comparing the code before the breaking commit and this branch you see that it now only adds the "ignore errors" flag and routes the
doordie()calls through a little helper that checks that flag. @schmonz verified that his usecase with the installed version now works.