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.

Sign up for GitHub

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

Jump to bottom

POP3 is broken #2756

Merged
merged 6 commits into from
Apr 8, 2023
Merged

POP3 is broken #2756

merged 6 commits into from
Apr 8, 2023

Conversation

nextgens
Copy link
Contributor

@nextgens nextgens commented Apr 8, 2023 •
edited
Loading

What type of PR?

bug-fix

What does this PR do?

Add a test to show it's broken, then fix it.

Related issue(s)

Prerequisites

Before we can consider review and merge, please make sure the following list is done and checked.
If an entry in not applicable, you can check it or remove it from the list.

  • In case of feature or enhancement: documentation updated accordingly
  • Unless it's docs or a minor change: add changelog entry file.

@nextgens nextgens added priority/p0 Critical bug without workaround / Must have type/bug Bug. Not working as intended labels Apr 8, 2023
@mergify
Copy link
Contributor

mergify bot commented Apr 8, 2023

Thanks for submitting this pull request.
Bors-ng will now build test images. When it succeeds, we will continue to review and test your PR.

bors try

Note: if this build fails, read this.

bors bot added a commit that referenced this pull request Apr 8, 2023
@bors
Try #2756:
50f435c
@nextgens
Copy link
Contributor Author

nextgens commented Apr 8, 2023

bors try

@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Already running a review

@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Build failed:

@nextgens
Copy link
Contributor Author

nextgens commented Apr 8, 2023

bors try

bors bot added a commit that referenced this pull request Apr 8, 2023
@bors
Try #2756:
be4e441
@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Build failed:

@nextgens
Copy link
Contributor Author

nextgens commented Apr 8, 2023

bors try

bors bot added a commit that referenced this pull request Apr 8, 2023
@bors
Try #2756:
e63838c
@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Build failed:

@nextgens
Copy link
Contributor Author

nextgens commented Apr 8, 2023

bors try

bors bot added a commit that referenced this pull request Apr 8, 2023
@bors
Try #2756:
5c965f0
@nextgens
Copy link
Contributor Author

nextgens commented Apr 8, 2023

bors try

@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Already running a review

@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Build failed:

@nextgens
Copy link
Contributor Author

nextgens commented Apr 8, 2023

bors try

bors bot added a commit that referenced this pull request Apr 8, 2023
@bors
Try #2756:
d83f8d4
@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Build failed:

@nextgens
Copy link
Contributor Author

nextgens commented Apr 8, 2023

bors try

bors bot added a commit that referenced this pull request Apr 8, 2023
@bors
Try #2756:
7228132
@nextgens nextgens requested review from ghostwheel42 and Diman0 April 8, 2023 09:38
@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Build succeeded:

Diman0
Diman0 previously requested changes Apr 8, 2023
View reviewed changes
Copy link
Member

@Diman0 Diman0 left a comment

Choose a reason for hiding this comment

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

With the suggested changes it should be fine. The delimiter must be random to make sure an attacker cannot put the delimiter in the newsfragment and still have the ability to execute random code.

I tested it here:
https://github.com/Diman0/Mailu_Fork/actions/runs/4644712577/jobs/8220143961
https://github.com/Diman0/Mailu_Fork/releases/tag/1.9.29

@mergify mergify bot dismissed Diman0’s stale review April 8, 2023 10:48

Pull request has been modified.

@nextgens
Copy link
Contributor Author

nextgens commented Apr 8, 2023

bors try

bors bot added a commit that referenced this pull request Apr 8, 2023
@bors
Try #2756:
5b935b3
@nextgens nextgens requested a review from Diman0 April 8, 2023 10:48
@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Build succeeded:

Diman0
Diman0 previously requested changes Apr 8, 2023
View reviewed changes
Copy link
Member

@Diman0 Diman0 left a comment

Choose a reason for hiding this comment

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

I think we are almost there. There are only a couple of tests that miss a non-zero exit code in case the tests succeed (while they should always fail)

tests/compose/core/05_connectivity.py Outdated
conn.ehlo()
conn.login(username, password)
print(f'Authenticating to smtp://{username}:{password}@{server}:25/ worked without STARTTLS!')
os.exit(104)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
os.exit(104)
os.exit(106)

due to the above changes this number must be bumped to make it unique.

@mergify mergify bot dismissed Diman0’s stale review April 8, 2023 11:24

Pull request has been modified.

@Diman0
Copy link
Member

Diman0 commented Apr 8, 2023

bors try

bors bot added a commit that referenced this pull request Apr 8, 2023
@bors
Try #2756:
849f96a
@Diman0 Diman0 added the type/backport Automatic backport this PR to the current stable release label Apr 8, 2023
@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

try

Build succeeded:

Diman0
Diman0 approved these changes Apr 8, 2023
View reviewed changes
Copy link
Member

@Diman0 Diman0 left a comment

Choose a reason for hiding this comment

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

BB8 Thumbs Up

@mergify
Copy link
Contributor

mergify bot commented Apr 8, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 8, 2023

Build succeeded:

@bors bors bot merged commit 1b7de12 into Mailu:master Apr 8, 2023
@mergify mergify bot mentioned this pull request Apr 8, 2023
Merged
bors bot added a commit that referenced this pull request Apr 8, 2023
@bors @nextgens
Merge #2758
661e3ef 
2758: POP3 is broken (backport #2756) r=mergify[bot] a=mergify[bot]

This is an automatic backport of pull request #2756 done by [Mergify](https://mergify.com).


---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the [documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- ``@Mergifyio` refresh` will re-evaluate the rules
- ``@Mergifyio` rebase` will rebase this PR on its base branch
- ``@Mergifyio` update` will merge the base branch into this PR
- ``@Mergifyio` backport <destination>` will backport this PR on `<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>

Co-authored-by: Florent Daigniere <nextgens@freenetproject.org>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Diman0 Diman0 Diman0 approved these changes

@ghostwheel42 ghostwheel42 Awaiting requested review from ghostwheel42

Assignees
No one assigned
Labels
priority/p0 Critical bug without workaround / Must have type/backport Automatic backport this PR to the current stable release type/bug Bug. Not working as intended
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Postfix master.cf override broken due to missing new line Retrieving messages via pop3 fails
2 participants
@nextgens @Diman0