RFR: 8358688: HttpClient: Simplify file streaming in RequestPublishers.FilePublisher [v2]

Daniel Fuchs dfuchs at openjdk.org
Fri Jun 6 08:23:55 UTC 2025


On Fri, 6 Jun 2025 07:44:33 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:

>> Placing `isRegularFile()` check to `create()` indeed was my initial attempt too. Though that resulted in a behavior change: Earlier the regular file check was performed by `FIS::new` at subscription, hence, `NSFE` (yes, `FIS::new` throws `NSFE` when passed a not-regular file, e.g., a directory) was thrown at `subscribe()`. Moving this check from `subscribe()` to `create()` breaks the `RelayingPublishers` test, since failure gets moved from subscription-time to [reactive pipeline] assembly-time.
>> 
>> Would you prefer me to
>> 
>> 1. keep things as is,
>> 2. move `isRegularFile()` to `create()`, or
>> 3. move `isRegularFile()` to `create()` and file a CSR?
>> 
>> Note that I don't think we can avoid the `NSFE`-to-`FNFE` translation in `subscribe()`, since, whatever earlier check we do before `Files::newIS`, file might get changed afterwards, and `NSFE` can still be thrown at the `Files::newIS` invocation.
>
>> I believe the old code was not always throwing `FileNotFoundException` - but might have thrown `NoSuchFileException` if using a non-default filesystem.
> 
> @dfuch, correct. If `Path` is associated with a file system that doesn't support `Path::toFile`<sup>1</sup>, earlier users would get `Files::newIS` exceptions (which were indeed undocumented), whereas others would get `FIS::new` exceptions.
> 
> <sup>1</sup> Allow me to note that I intentionally avoid using "default file system" term, since, IMHO, that distracts us from the problem: file systems that don't support `Path::toFile`. The "default file system" is required to support that feature, but there can be other file systems that support that too.

OK

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25662#discussion_r2131745751


More information about the net-dev mailing list