On Fri, 6 Jun 2025 08:11:35 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:

>> Simplifies content streaming for `HttpRequest.BodyPublishers::ofFile(Path)`, 
>> which delegates to `RequestPublishers.FilePublisher::create(Path)`. This 
>> effectively replaces all usages of `FileInputStream::new` in `HttpClient` 
>> with `Files::newInputStream(Path)`. See the JBS ticket description for the 
>> full story.
>> 
>> Also removing certain test files which were rendered redundant after 
>> `SecurityManager` removal. We do this in this PR, since tests were added due 
>> to touched lines.
>> 
>> `tier1,2` results on b5f3c8dc6f4 are attached to the ticket.
>
> Volkan Yazici has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add back removed SM tests

The new version LGTM. I wonder if we should add `@bug 8358688` to the various 
FilePublisher test - or just consider the fix as noreg-cleanup. On the one hand 
those tests might have failed if you hadn't catched and transformed NSFE. On 
the other hand they should pass whether this fix is present or not...

src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java 
line 269:

> 267:                 // The old code was using `FileInputStream::new`, which 
> throws `FNFE` if file doesn't exist.
> 268:                 // Preserving that behaviour after migrating to 
> `Files::newInputStream`:
> 269:                 t = new FileNotFoundException(path + " (No such file or 
> directory)");

I wonder if we should keep `nsfe` as the cause. Does it contain anything 
interesting in the stack trace that could help debugging if it gets 
unexpectedly thrown?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/25662#issuecomment-2948485426
PR Review Comment: https://git.openjdk.org/jdk/pull/25662#discussion_r2131758853

Reply via email to