Rfc2045 decode

[tbrk]

Some XML-RPC servers break Base64 encoded data into lines of 76 characters long (see, e.g., the Python xmlrpc library). In these cases the Base64.decode_exn function raises an exception Invalid_argument "Malformed input".

I propose, in this pull request, to decode using the Base64_rfc2045 module which handles newlines. This module also decodes base64 strings without line breaks (in "dangerous" mode) and thus supports existing behavior.

[mseri]

I think this should be made configurable, maybe it is better to see first how this is going to be addressed in ocaml-base64

[tbrk]

What would you think about allowing the caller to provide a decode function (defaulting to Base64.decode), either by threading an argument down to the point where decoding happens or using a module-level reference to set the decoding function?

[tbrk]

As it stands it's not possible to use ocaml-rpc to interact with a server written in Python (using the xmlrpc library) that returns a base64-encoded value.

[mseri]

I understand the reason behind the PR. As per the solution, I think it would be fine to provide an optional decoder parameter, which uses the current one by default but allows you to pass in anything else. We could also expose the current Base64_rfc2045 one, so that there is already the alternative available

[tbrk]

OK. I agree with you that some kind of parameterization is necesary to handle all the different cases (no line breaks, line breaks every 76 characters with \r\n, line breaks every 76 characters with \n, etc.). I'll draft a solution and include the suggested comment and documentation. Thanks @mseri.
My current suggestion is a nasty hack.

[mseri]

Thanks for the contribution

[tbrk]

This commit allows callers of Xmlrpc to override the function used to decode base64 values. It has the advantage of externalizing the decoding strategy. I guess the correct place for base64 decoding functions is in ocaml-base64.
I did not update Rpc and Rpcmarshal, but I can do this if you would like. I'm not sure how useful the override option is in these cases.

[tbrk]

I've updated my proof of concept to use the proposed interface.

[mseri]

Suggested change

	(* The parsing functions make it possible to specify the routine used to
	(** The parsing functions make it possible to specify the routine used to

Let’s have this in the documentation

[mseri]

Suggested change

	let rec of_xml ?callback ?base64_decode accu input =
	let rec of_xml ?callback ?base64_decoder accu input =

I am bikeshedding here, but I would usebase64_decoder as parameter name

[mseri]

Thanks. I like this.

I think it makes sense to leave it only on the xmlrpc module right now. I would copy your little implementation of the base 64 decoder and maybe add a comment in the Readme or the documentation showing it and why it can be useful when communicating with python. But I can do that myself before releasing, after this is merged.

thanks a lot for the contributions

[tbrk]

Thanks for your suggestions and maintainership! I think the final result we arrived at is reasonable and makes the library more useful.

[mseri]

Thanks a lot for your patience and help!

[mseri]

I will prepare a new release by the end of the week