Replace OpenURI with Net::HTTP

[danielfone]

Hi @philnash,

I wonder if you would consider this change to improve the security of the gem.

This replaces OpenURI with the slightly more verbose but less intrusive Net:HTTP interface. OpenURI monkey patches the Kernel.open method in a possibly unsafe way, which could be surprising behavior for a gem like this.

This also improves the performance to actually stream the response, since as far as I can tell, io.each_line reads the entire response first.

request_options have been retained as headers, but other configuration options that were supported by OpenURI need to be considered and the docs updated. This is unlikely to be possible in a 100% backwards compatible way.

Before I add any new request_options docs, I thought I'd run this by you.

Thanks,

Daniel

[philnash]

Hey, sorry for the slow response. I'm totally up for making this change. Is this also going to make responses faster since it is streaming the response?

[danielfone]

Thanks @philnash

Is this also going to make responses faster since it is streaming the response?

I believe so, but this isn't something I've actually tested. Looking at the code for open-uri, this seems like a more efficient way to do things, but it's a little hard to unravel.

What's your feeling on migrating the request_options? The equivalent options that open supports are spread across a bunch of the Net::HTTP methods. Is backwards compatibility worth thinking about at all? What configuration options are people likely to need do you think? Are request headers sufficient, or should we give them access to all the Net::HTTP options in some way.

[philnash]

I am not too worried about backwards compatibility. More than happy to change the API to allow for this update and to cut a new major version for it. It wouldn't take much to upgrade.

The API itself doesn't require anything more than a User Agent. However, it's possible for the future that people will want to download the dataset and use this gem against their own version of the API that may require other headers, so I think it's reasonable to allow users to set any HTTP header.

read_timeout and open_timeout are important too, so that users can limit the time they could wait for the API. These options are also called out and used in the devise plugin.

Those are the options called out by the gem so far. Are there any others you think would be important that open-uri wasn't making available?

[danielfone]

@philnash I've pushed a few new commits which update the request_options hash to use a specific key to configure http request headers. This seemed like the simplest change to make. I've also updated the documentation.