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

Use java.nio to load filter files #2684

Merged
merged 3 commits into from
Nov 11, 2023

Conversation

gtoison
Copy link
Contributor

@gtoison gtoison commented Nov 7, 2023

java.io.FileInputStream seems to be running into issues when trying to load files with non-ascii charatecter in the file name.
Other similar issues point to this happening on a Mac, so it might be dependent on the combination of OS/JDK version/user settings.
Although the test did not reproduce the problem I think that using java.nio should help.
Note that I'm using FileSystems.getDefault().getPath(...) instead of Path.of(...) because it is only available since JDK 11.

Originally reported here:
JetBrains/spotbugs-intellij-plugin#1492

Other links discussing similar problems:
https://stackoverflow.com/questions/22775758/java-io-file-accessing-files-with-invalid-filename-encodings
https://ogris.de/howtos/java-utf8-filenames.html
https://stackoverflow.com/questions/14171565/java-read-write-unicode-utf-8-filenames-not-contents

java.io.FileInputStream seems to be running into issues when trying to
load files with non-ascii charatecter in the file name.
Other similar issues point to this happening on a Mac, so it might be
dependent on the combination of OS/JDK version/user settings
Originally reported here:
JetBrains/spotbugs-intellij-plugin#1492
@gtoison gtoison marked this pull request as ready for review November 9, 2023 07:38
@gtoison gtoison changed the title test: load a filter with non-ascii characters in file name Use java.nio to load filter files Nov 9, 2023
Copy link
Collaborator

@JuditKnoll JuditKnoll left a comment

Choose a reason for hiding this comment

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

Tried to reproduce the problem with the provided test without the solution both on a Windows and on a Linux machine (using OpenJDK 17 on both), but I couldn't, the test was successful before using java.nio, and after as well.

If using java.nio solves the problem, it may be worth to look at and consider using java.nio in other places operating with Files as well to prevent similar issues.

@@ -9,6 +9,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.

### Fixed
- Fixed false positive UPM_UNCALLED_PRIVATE_METHOD for method used in JUnit's MethodSource ([[#2379](https://github.com/spotbugs/spotbugs/issues/2379)])
- Use java.nio to load filter files ([[#2684](https://github.com/spotbugs/spotbugs/pull/2684)])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure, you wanted to refer the PR here and not the fixed issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue in the IntelliJ plugin, since Jetbrains does not seem to be actively maintaining it I think it will take a while until the plugin gets the updated version

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this exact instance, I get it. However, I couldn't find the general logic yet, when exactly is referenced the issue, and when the PR, when there are no links? The links seems the same, when looking at the preview of the file (not the raw version), which may be a bit confusing for the reader. Until this point, I only refered the fixed issues. Could you please clarify this to me? (It's a bit off topic here, sorry about it.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know either to be honest, but as a user I would expect a link to something giving some context as to what was changed and why

@hazendaz hazendaz self-assigned this Nov 11, 2023
@hazendaz hazendaz merged commit 95f1754 into spotbugs:master Nov 11, 2023
6 checks passed
@hazendaz hazendaz added this to the SpotBugs 4.8.2 milestone Dec 18, 2023
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.

None yet

4 participants