Closed valenpo closed 1 year ago
Attention: 1 lines
in your changes are missing coverage. Please review.
Files | Coverage Δ | |
---|---|---|
...subethamail/smtp/internal/command/BdatCommand.java | 79.62% <50.00%> (-1.14%) |
:arrow_down: |
... and 1 file with indirect coverage changes
:loudspeaker: Thoughts on this report? Let us know!.
Please remove those other commits so we can merge the other stuff. They should be in another pr, another discussion.
Dave, I m not sure what I need to do exactly
Сб, 30 сент. 2023 г. в 06:07, Dave Moten @.***>:
Please remove those other commits so we can merge the other stuff. They should be in another pr, another discussion.
— Reply to this email directly, view it on GitHub https://github.com/davidmoten/subethasmtp/pull/109#issuecomment-1741641829, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIMLQ7ZDTVWMKRFEXSEJITX46EFHANCNFSM6AAAAAA5HUNWTA . You are receiving this because you authored the thread.Message ID: @.***>
I was about to merge the bdat stuff and then you added inputstream changes. Please remove those inputstream commits (which I'm a bit doubtful about anyway) for discussion in another pr. I suggest you apply use of BufferedInputStream in one PR then talk about the exact benefits of the NonCloseableInputStream work in an issue or another PR. Thanks.
Done. Sorry for mess up.
On 1 Oct 2023, at 23:12, Dave Moten @.***> wrote:
I was about to merge the bdat stuff and then you added inputstream changes. Please remove those inputstream commits (which I'm a bit doubtful about anyway) for discussion in another pr. I suggest you apply use of BufferedInputStream in one PR then talk about the exact benefits of the NonCloseableInputStream work in an issue or another PR. Thanks.
— Reply to this email directly, view it on GitHub https://github.com/davidmoten/subethasmtp/pull/109#issuecomment-1742188198, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIMLQ5GPJNFT7CRGEMFMWTX5HFDBANCNFSM6AAAAAA5HUNWTA. You are receiving this because you authored the thread.
Regards, Valentin Popov
I think reason is, that BufferedInputStream will close decorated stream. All test’s pass.
On 2 Oct 2023, at 14:45, Dave Moten @.***> wrote:
@davidmoten requested changes on this pull request.
In src/main/java/org/subethamail/smtp/internal/command/BdatCommand.java https://github.com/davidmoten/subethasmtp/pull/109#discussion_r1342583802:
@@ -36,7 +39,7 @@ public void execute(String commandString, Session sess) return; }
- InputStream stream = new BdatInputStream(sess.getRawInput(), sess, bdat.size, bdat.isLast);
- InputStream stream = new BdatInputStream(new BufferedInputStream(sess.getRawInput(), BUFFER_SIZE), sess, bdat.size, bdat.isLast); Sorry, I wasn't clear enough. Please remove all stream stuff from this PR. Put this sort of change in another PR. In the back of my mind there was some reason that this didn't happen earlier, can't remember but I'll look at it closer in next PR.
— Reply to this email directly, view it on GitHub https://github.com/davidmoten/subethasmtp/pull/109#pullrequestreview-1652580242, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIMLQZPHDWBLMWYJDX74KTX5KSOTANCNFSM6AAAAAA5HUNWTA. You are receiving this because you authored the thread.
Regards, Valentin Popov
Thanks!
On 3 Oct 2023, at 10:47, Dave Moten @.***> wrote:
@davidmoten approved this pull request.
— Reply to this email directly, view it on GitHub https://github.com/davidmoten/subethasmtp/pull/109#pullrequestreview-1654547708, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIMLQ3GS5OWOAQZC2JY7GTX5O7I7AVCNFSM6AAAAAA5HUNWTCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNJUGU2DONZQHA. You are receiving this because you authored the thread.
Regards, Valentin Popov
Great, thanks @valenpo, that's in
Any BDAT command sent after the BDAT LAST is illegal and MUST be replied to with a 503 "Bad sequence of commands" reply code