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

Incapsulate Session.getRawInput() to prevent close Socket#InputStream #113

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

valenpo
Copy link

@valenpo valenpo commented Oct 3, 2023

All InputStream's that decorate Socket.inputStream could be used in try block safe.
Close of Socket InputStream happens on Session class.

@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Attention: Patch coverage is 96.29630% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 71.02%. Comparing base (e4c24f4) to head (8673475).
Report is 19 commits behind head on master.

Current head 8673475 differs from pull request most recent head 87c94b8

Please upload reports for the commit 87c94b8 to get more accurate results.

Files Patch % Lines
...subethamail/smtp/internal/command/DataCommand.java 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #113      +/-   ##
============================================
+ Coverage     70.92%   71.02%   +0.09%     
- Complexity      463      466       +3     
============================================
  Files            72       72              
  Lines          2373     2381       +8     
  Branches        248      248              
============================================
+ Hits           1683     1691       +8     
  Misses          572      572              
  Partials        118      118              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Bumps [com.github.davidmoten:guava-mini](https://github.com/davidmoten/guava-mini) from 0.1.6 to 0.1.7.
- [Release notes](https://github.com/davidmoten/guava-mini/releases)
- [Commits](davidmoten/guava-mini@0.1.6...0.1.7)

---
updated-dependencies:
- dependency-name: com.github.davidmoten:guava-mini
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
@davidmoten
Copy link
Owner

excuse the delay, what do we gain from this change? Do we have a resource leak? I don't believe we do so I don't see any benefit (reduces false positives from code quality checkers perhaps?).

@valenpo
Copy link
Author

valenpo commented Oct 13, 2023

excuse the delay, what do we gain from this change? Do we have a resource leak? I don't believe we do so I don't see any benefit (reduces false positives from code quality checkers perhaps?).

we encapsulate the stream in the place where it should be closed. Otherwise, any other user who uses the recommended try catch block will fall into the trap that I fell into.

We are also correcting the TODO, which says that the stream should be closed in the place from where we open it. In the current implementation, it is necessary to keep in mind where it is necessary and where it is not necessary to close the streams, this is not very nice and correct. And an open door for future mistakes.

Bumps [org.jacoco:jacoco-maven-plugin](https://github.com/jacoco/jacoco) from 0.8.10 to 0.8.11.
- [Release notes](https://github.com/jacoco/jacoco/releases)
- [Commits](jacoco/jacoco@v0.8.10...v0.8.11)

---
updated-dependencies:
- dependency-name: org.jacoco:jacoco-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
@valenpo
Copy link
Author

valenpo commented Oct 18, 2023

@davidmoten Hi. Same way tika incapsulate stream that should not be closed. Any update?

@davidmoten
Copy link
Owner

Sorry Valentin, working two jobs at the moment and am short on headspace. What I plan to do is look at your suggestion in the IDE and give you some feedback then.

@valenpo
Copy link
Author

valenpo commented Oct 18, 2023 via email

@davidmoten
Copy link
Owner

Can you drop the changes to the pom please (jacoco and guava-mini)? And rename convert to read?

@valenpo
Copy link
Author

valenpo commented Oct 30, 2023 via email

dependabot bot and others added 4 commits October 30, 2023 20:58
Bumps [com.github.davidmoten:guava-mini](https://github.com/davidmoten/guava-mini) from 0.1.6 to 0.1.7.
- [Release notes](https://github.com/davidmoten/guava-mini/releases)
- [Commits](davidmoten/guava-mini@0.1.6...0.1.7)

---
updated-dependencies:
- dependency-name: com.github.davidmoten:guava-mini
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.jacoco:jacoco-maven-plugin](https://github.com/jacoco/jacoco) from 0.8.10 to 0.8.11.
- [Release notes](https://github.com/jacoco/jacoco/releases)
- [Commits](jacoco/jacoco@v0.8.10...v0.8.11)

---
updated-dependencies:
- dependency-name: org.jacoco:jacoco-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.apache.maven.plugins:maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.6.0 to 3.6.2.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.6.0...maven-javadoc-plugin-3.6.2)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
@valenpo
Copy link
Author

valenpo commented Nov 10, 2023

@davidmoten hi. Any chance to look?

Bumps [org.apache.maven.plugins:maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.6.2 to 3.6.3.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.6.2...maven-javadoc-plugin-3.6.3)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.apache.maven.plugins:maven-compiler-plugin](https://github.com/apache/maven-compiler-plugin) from 3.11.0 to 3.12.0.
- [Release notes](https://github.com/apache/maven-compiler-plugin/releases)
- [Commits](apache/maven-compiler-plugin@maven-compiler-plugin-3.11.0...maven-compiler-plugin-3.12.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-compiler-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.apache.maven.plugins:maven-compiler-plugin](https://github.com/apache/maven-compiler-plugin) from 3.12.0 to 3.12.1.
- [Release notes](https://github.com/apache/maven-compiler-plugin/releases)
- [Commits](apache/maven-compiler-plugin@maven-compiler-plugin-3.12.0...maven-compiler-plugin-3.12.1)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-compiler-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.apache.maven.plugins:maven-compiler-plugin](https://github.com/apache/maven-compiler-plugin) from 3.12.1 to 3.13.0.
- [Release notes](https://github.com/apache/maven-compiler-plugin/releases)
- [Commits](apache/maven-compiler-plugin@maven-compiler-plugin-3.12.1...maven-compiler-plugin-3.13.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-compiler-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.jacoco:jacoco-maven-plugin](https://github.com/jacoco/jacoco) from 0.8.11 to 0.8.12.
- [Release notes](https://github.com/jacoco/jacoco/releases)
- [Commits](jacoco/jacoco@v0.8.11...v0.8.12)

---
updated-dependencies:
- dependency-name: org.jacoco:jacoco-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.apache.maven.plugins:maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.6.3 to 3.7.0.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.6.3...maven-javadoc-plugin-3.7.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.apache.maven.plugins:maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.7.0 to 3.8.0.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.7.0...maven-javadoc-plugin-3.8.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.apache.maven.plugins:maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.8.0 to 3.10.0.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.8.0...maven-javadoc-plugin-3.10.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.apache.maven.plugins:maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.10.0 to 3.10.1.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.10.0...maven-javadoc-plugin-3.10.1)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
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.

2 participants