HTTP server, InputStream API skip() reset() read(), restrictions for Content Filter Chain usage #51
Description
This is a follow-up discussion to this Pull Request (when NanoHTTPD was replaced with AndroidAsync): #50
Comments reproduced here:
Hi @jccr I reviewed your latest commit and I noticed your addition in InputStream skip() and reset() in the non-isRange case. Note that with encrypted media, this is actually likely to result in incorrect stream positioning, as the "Content Filter Chain" mechanism is designed only to handle read() operations. See here:
https://github.com/readium/readium-sdk/blob/develop/ePub3/ePub/filter_chain_byte_stream.cpp#L71
However, I do not think that this is a ship-stopper, because as far as I can tell the AndroidAsync HTTP server does not invoke these methods anyway.
Ultimately, perhaps we could mimic what I did with NanoHTTPD server: implement a subset of InputStream to restrict usage to only Readium's required stream-access methods (I'm not sure about AndroidAsync's internals, but perhaps this would allow getting rid of the no-parameter read() method as well). Alternatively, perhaps we could just throw NotImplementedException or something along these lines to make it clear that the "Content Filter Chain" backend does not support arbitrary seeking, only a well-defined read-based protocol.
@nleme am I getting this right? It would be great to hear your thoughts before we merge, just to make sure we're not missing anything glaringly obvious :)
PS: regarding InputStream skip() usage in AndroidAsync, note that the returned value does not necessarily reflect the position in the raw underlying ByteStream in the case of encrypted media. However we can easily work around that without modifying AndroidAsync's source code:
if (start != inputStream.skip(start))
throw new StreamSkipException("skip failed to skip requested amount");
As for Util.pump(): only this read() method signature is used:
int read = is.read(b.array(), 0, (int)toRead);
Conclusion: at a later date we will definitely be able to easily restrict the InputStream API that AndroidAsync consumes. For now, let's just merge to develop (pending Nelson's approval).