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

SamReaderFactory.open treats the same non-regular File and Path differently #1716

Closed
kshakir opened this issue Sep 29, 2024 · 2 comments · Fixed by #1717
Closed

SamReaderFactory.open treats the same non-regular File and Path differently #1716

kshakir opened this issue Sep 29, 2024 · 2 comments · Fixed by #1717

Comments

@kshakir
Copy link
Contributor

kshakir commented Sep 29, 2024

Description of the issue:

This issue is related to:

Describe your issue here.
When creating instances of SamInputResource, there are two overloads that take a java.nio.Path:

  1. public static SamInputResource of(final Path path)
  2. public static SamInputResource of(final Path path, Function<SeekableByteChannel, SeekableByteChannel> wrapper)

As of #1124, these two methods have very different behaviors on non-regular files. Overload 1 partially handles the issues described in #1084, while overload 2 does not.

To choose between the overloads, code such as in #1494 checks if the caller has passed in a null wrapper to call overload 1 or 2. Direct link: https://github.com/samtools/htsjdk/pull/1494/files#diff-1001d1e3b784b6fa1bab848edafb7430f4f031dd17fa0bee7a5482880d2a1a7aR159-R161

However, as of HTSJDK 4.1.1, this same conditional logic is missing from the SamReaderFactory method public SamReader open(final Path path, Function<SeekableByteChannel, SeekableByteChannel> dataWrapper, Function<SeekableByteChannel, SeekableByteChannel> indexWrapper).

Unlike the code linked above in #1494, the SamReaderFactory.open method does not check for null wrappers. This ultimately means that non-regular files passed as java.io.File end up being treated differently than the exact same file passed as java.nio.file.Path.

Your environment:

  • version of htsjdk: 4.1.1
  • version of java: 21-tem
  • which OS: macOS 14.6.1

Steps to reproduce

Call:

File irregularFile = new File("/dev/stdin");
SamReader fileReader = SamReaderFactory.makeDefault().open(irregularFile);
SamReader pathReader = SamReaderFactory.makeDefault().open(irregularFile.toPath());
System.out.println(fileReader.getResourceDescription())
System.out.println(pathReader.getResourceDescription())

Expected behaviour

data=FILE:/dev/null;index=null
data=FILE:/dev/null;index=null

Actual behaviour

data=FILE:/dev/null;index=null
data=PATH:/dev/null;index=null

Workaround

While this is being fixed, you are able to write your own open method that works correctly. Something like:

public static SamReader open(final Path path, final SamReaderFactory factory) {
    final SamInputResource r = SamInputResource.of(path);
    final Path indexMaybe = SamFiles.findIndex(path);
    if (indexMaybe != null) r.index(indexMaybe);
    return factory.open(r);
}
@kshakir
Copy link
Contributor Author

kshakir commented Oct 1, 2024

Btw, if it makes one feel better Java doesn't treat non-regular files the same way either.

File.newInputStream cannot be used with linux character device files

via https://bugs.openjdk.org/browse/JDK-8233451

@lbergelson
Copy link
Member

Oh, you found a matching bug report. I couldn't find it.

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 a pull request may close this issue.

2 participants