On Fri, Sep 11, 2015 at 11:05 AM, Stefan Fuhrmann < stefan.fuhrm...@wandisco.com> wrote:
> On Fri, Sep 11, 2015 at 11:01 AM, Branko Čibej <br...@apache.org> wrote: > >> On 10.09.2015 23:50, Evgeny Kotkov wrote: >> > Stefan Fuhrmann <stef...@apache.org> writes: >> > >> > + * Special files like STDIN will have no flags set at all. In that >> case, >> > + * we can't filter and must allow any operation - which may then >> fail at >> > + * the APR level further down the stack. >> > + */ >> > + apr_uint32_t flags = apr_file_flags_get(file); >> > + svn_boolean_t supports_read = (flags == 0) || (flags & APR_READ); >> > + svn_boolean_t supports_write = (flags == 0) || (flags & APR_WRITE); >> > >> > Files corresponding to the standard I/O streams actually have the >> appropriate >> > APR_READ / APR_WRITE flags set starting from APR 1.3 — see, for example, >> > apr_file_open_flags_stderr() in [1]. Hence, the (flags == 0) check is >> going >> > to return false for them. >> > >> >> Note that this check is not perfect for arbitrary APR file handles >> >> (may enable more functions than the handle actually supports) but >> >> works correctly for our STD* streams and the files opened through >> >> our svn_io_* API. >> > The actual problem, to my mind, is that relying on flags to determine >> if the >> > file allows reading or writing, is fragile. There are examples of >> apr_file_t's >> > that don't have the corresponding flags, but still allow reading and >> writing, >> > and svn_stream_from_aprfile2() is going to break for them. >> > >> > One example would be apr_file_pipe_create() on Unix that sets >> APR_INHERIT >> > flag on the created pipe [2]. Another example is creating a pipe on >> Windows >> > that currently initializes flags to zero [3]. >> >> I've reviewed the JavaHL code again and it indeed appears that this is >> caused by the difference in implementations of apr_file_pipe_create_ex() >> on Windows and elsewhere. The bindings code itself is not >> platform-specific; see TunnelContext in >> subversion/bindings/javahl/native/OperationContext.cpp. Those pipes get >> wrapped into streams deep in the RA layer. >> >> One could argue that apr_file_pipe_create is broken since it doesn't set >> the appropriate flags on the input and output handles, but that doesn't >> really help make this particular instance in the bindings code work. >> > > O.k. Then I'll simply revert. > Done in r1702410. -- Stefan^2.
