close the stream parse(Path) and parse(URL) open on construction failure

[rootvector2]

parse(Path) and parse(URL) allocate the input stream themselves (Files.newInputStream(path), url.openStream()) and hand it to the CSVParser constructor. When createHeaders() rejects the header, the constructor throws before the caller receives a parser, so there is no Closeable to manage with try-with-resources and the stream the library opened is unreachable. Found while checking what happens to the handle when a header fails validation.

Reworked per review: the caller owns what the caller allocates, and the testTryWithResourcesParse* tests in master cover that case, so this no longer touches caller-supplied readers or streams. Only the two methods that open a stream internally now close it on failure, with a close failure attached as suppressed so the original error still surfaces.

testParseUrlClosesInputStreamWhenHeaderIsInvalid fails on master (expected: <true> but was: <false>) and passes with this change; the testTryWithResourcesParse* tests still pass. Full default mvn build is green.

• Read the contribution guidelines for this project.
• Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
• I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
• Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
• Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

[garydgregory]

Hello @rootvector2

This one feels wrong to me.

In git master, please find the new CSVParserTest.testTryWithResourcesParse*

• testTryWithResourcesParseInputStreamWhenHeaderIsInvalid
• testTryWithResourcesParseReaderWhenHeaderIsInvalid

It is the responsibility of the code that allocates a resource to close it.

If the the resource is managed with a try-with-resources block (see the new tests), the resource is closed for this use case. I switched the PR to draft for this reason.

Please let me know how you see it.

TY!

[rootvector2]

Agreed on caller-supplied readers; the new testTryWithResourcesParse* tests cover that case, so I dropped that part.

The case I was after is narrower: parse(Path) and parse(URL) allocate the stream themselves via Files.newInputStream(path) and url.openStream(). When createHeaders() throws, the caller never gets a parser back, so there is nothing to put in a try-with-resources block and the stream the library opened is unreachable. Applying the same rule, the method that allocated it now closes it on failure; caller-supplied readers and streams are untouched. Reworked the PR that way and rebased onto master. Your new tests still pass, and testParseUrlClosesInputStreamWhenHeaderIsInvalid fails without the change.

[garydgregory]

@rootvector2 On macOS, the test testParsePathClosesInputStreamWhenHeaderIsInvalid passes without changes to main, so it doesn't show anything to fix.

[garydgregory]

@rootvector2
I brought this in with a refactored implementation but a better test for Path would be good, or not, up to you ;)