Raise EPIPE for all unseekable channels #7415
Conversation
This relates to the discussion in discourse/mini_mime#38 where the proposed patch must work around JRuby's silent seek failure for some types of resources (at least classloader resources). The patch here will now raise EPIPE for all NIO channels that are not seekable through some means. It is rather aggressive given the old logic that only raised if a channel was not seekable AND not selectable (for reasons long lost).
|
The silent failure was originally introduced as a workaround for #3399. I'm all for getting rid of it, but we should probably make sure there's no regression for that issue. |
|
FWIW, I did a bit of source digging: it looks like the error in #3399 was triggered by this line in The "fix" in a4cc5ee made the seek fail silently, which is probably fine in the common case where there's only one range that starts at position 0, but it also probably made actual HTTP range requests with a non-zero start offset silently return incorrect data when the underlying resource is non-seekable. Or, rather, it probably allowed such requests to continue breaking silently, since I doubt they ever worked correctly for non-seekable resources in the first place. It would probably be worth filing a Rack issue about this, but in the mean time we also probably shouldn't break the current Rack code until it's been fixed (in all supported Rack versions). I wonder if it would be possible to detect no-op seeks that don't actually change the current file position (e.g. by maintaining our own file position counter) and only silently ignoring those. 🤔 |
|
@ikaronen-relex Thanks for that research! I agree we should not go forward with the breaking change for now, but I'm unsure how to proceed. It seems logical that classloader resources should not be seekable, since the only way to access them is through an InputStream that does not support positioning. However, we don't want to break rack applications that might expect the silent seek failure as you discovered. |
This relates to the discussion in discourse/mini_mime#38 where the proposed patch must work around JRuby's silent seek failure for some types of resources (at least classloader resources).
The patch here will now raise EPIPE for all NIO channels that are not seekable through some means. It is rather aggressive given the old logic that only raised if a channel was not seekable AND not selectable (for reasons long lost).