Skip to content

Commit

Permalink
Convert library to use built-in Net::HTTP
Browse files Browse the repository at this point in the history
Moves the library off of Faraday and over onto the standard library's
built-in `Net::HTTP` module. The upside of the transition is that we
break away from a few dependencies that have caused us a fair bit of
trouble in the past, the downside is that we need more of our own code
to do things (although surprisingly, not that much more).

The biggest new pieces are:

* `ConnectionManager`: A per-thread class that manages a connection to
  each Stripe infrastructure URL (like `api.stripe.com`,
  `connect.stripe.com`, etc.) so that we can reuse them between
  requests. It's also responsible for setting up and configuring new
  `Net::HTTP` connections, which is a little more heavyweight
  code-wise compared to other libraries. All of this could have lived in
  `StripeClient`, but I extracted it because that class has gotten so
  big.

* `MultipartEncoder`: A class that does multipart form encoding for file
  uploads. Unfortunately, Ruby doesn't bundle anything like this. I
  built this by referencing the Go implementation because the original
  RFC is not very detailed or well-written. I also made sure that it was
  behaving similarly to our other custom implementations like
  stripe-node, and that it can really upload a file outside the test
  suite.

There's some risk here in that it's easy to miss something across one of
these big transitions. I've tried to test out various error cases
through tests, but also by leaving scripts running as I terminate my
network connection and bring it back. That said, we'd certainly release
on a major version bump because some of the interface (like setting
`Stripe.default_client`) changes.
  • Loading branch information
brandur committed Jul 24, 2019
1 parent 267eae5 commit 97db02f
Show file tree
Hide file tree
Showing 18 changed files with 850 additions and 272 deletions.
6 changes: 6 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ Layout/CaseIndentation:
Layout/IndentArray:
EnforcedStyle: consistent

# This can be re-enabled once we're 2.3+ only and can use the squiggly heredoc
# operator. Prior to that, Rubocop recommended bringing in a library like
# ActiveSupport to get heredoc indentation, which is just terrible.
Layout/IndentHeredoc:
Enabled: false

Layout/IndentHash:
EnforcedStyle: consistent

Expand Down
17 changes: 13 additions & 4 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,28 @@
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

Layout/ClosingHeredocIndentation:
# As far as I can tell, this lint rule just doesn't work when it comes to
# heredoc in `it` blocks.
Exclude:
- 'test/**/*'

# Offense count: 20
Metrics/AbcSize:
Max: 53

# Offense count: 31
# Configuration parameters: CountComments, ExcludedMethods.
Metrics/BlockLength:
Max: 498
Max: 39

# Test `context` is a block and generates all kinds of false positives
# without exclusion.
Exclude:
- 'test/**/*'

# Offense count: 11
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 673
Max: 700

# Offense count: 12
Metrics/CyclomaticComplexity:
Expand Down
11 changes: 4 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,13 @@ Stripe::Charge.retrieve(
)
```

### Configuring a Client
### Accessing a response object

While a default HTTP client is used by default, it's also possible to have the
library use any client supported by [Faraday][faraday] by initializing a
`Stripe::StripeClient` object and giving it a connection:
Get access to response objects by initializing a client and using its `request`
method:

```ruby
conn = Faraday.new
client = Stripe::StripeClient.new(conn)
client = Stripe::StripeClient.new
charge, resp = client.request do
Stripe::Charge.retrieve(
"ch_18atAXCdGbJFKhCuBAa4532Z",
Expand Down Expand Up @@ -272,7 +270,6 @@ Update the bundled [stripe-mock] by editing the version number found in
[api-keys]: https://dashboard.stripe.com/account/apikeys
[connect]: https://stripe.com/connect
[curl]: http://curl.haxx.se/docs/caextract.html
[faraday]: https://github.com/lostisland/faraday
[idempotency-keys]: https://stripe.com/docs/api/ruby#idempotent_requests
[stripe-mock]: https://github.com/stripe/stripe-mock
[versioning]: https://stripe.com/docs/api/ruby#versioning
Expand Down
15 changes: 8 additions & 7 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ RuboCop::RakeTask.new

desc "Update bundled certs"
task :update_certs do
require "faraday"
require "net/http"
require "uri"

fetch_file "https://curl.haxx.se/ca/cacert.pem",
::File.expand_path("../lib/data/ca-certificates.crt", __FILE__)
Expand All @@ -23,14 +24,14 @@ end
# helpers
#

def fetch_file(url, dest)
def fetch_file(uri, dest)
::File.open(dest, "w") do |file|
resp = Faraday.get(url)
unless resp.status == 200
abort("bad response when fetching: #{url}\n" \
"Status #{resp.status}: #{resp.body}")
resp = Net::HTTP.get_response(URI.parse(uri))
unless resp.code.to_i == 200
abort("bad response when fetching: #{uri}\n" \
"Status #{resp.code}: #{resp.body}")
end
file.write(resp.body)
puts "Successfully fetched: #{url}"
puts "Successfully fetched: #{uri}"
end
end
4 changes: 3 additions & 1 deletion lib/stripe.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
# Stripe Ruby bindings
# API spec at https://stripe.com/docs/api
require "cgi"
require "faraday"
require "json"
require "logger"
require "net/http"
require "openssl"
require "rbconfig"
require "securerandom"
Expand All @@ -28,6 +28,8 @@
require "stripe/errors"
require "stripe/object_types"
require "stripe/util"
require "stripe/connection_manager"
require "stripe/multipart_encoder"
require "stripe/stripe_client"
require "stripe/stripe_object"
require "stripe/stripe_response"
Expand Down
122 changes: 122 additions & 0 deletions lib/stripe/connection_manager.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# frozen_string_literal: true

module Stripe
# Manages connections across multiple hosts which is useful because the
# library may connect to multiple hosts during a typical session (main API,
# Connect, Uploads). Ruby doesn't provide an easy way to make this happen
# easily, so this class is designed to track what we're connected to and
# manage the lifecycle of those connections.
#
# Note that this class in itself is *not* thread safe. We expect it to be
# instantiated once per thread.
#
# Note also that this class doesn't currently clean up after itself because
# it expects to only ever have a few connections. It'd be possible to tank
# memory by constantly changing the value of `Stripe.api_base` or the like. A
# possible improvement might be to detect and prune old connections whenever
# a request is executed.
class ConnectionManager
def initialize
@active_connections = {}
end

# Gets a connection for a given URI. This is for internal use only as it's
# subject to change (we've moved between HTTP client schemes in the past
# and may do it again).
#
# `uri` is expected to be a string.
def connection_for(uri)
u = URI.parse(uri)
connection = @active_connections[[u.host, u.port]]

if connection.nil?
connection = create_connection(u)

# TODO: what happens after TTL?
connection.start

@active_connections[[u.host, u.port]] = connection
end

connection
end

# Executes an HTTP request to the given URI with the given method. Also
# allows a request body, headers, and query string to be specified.
def execute_request(method, uri, body: nil, headers: nil, query: nil)
# Perform some basic argument validation because it's easy to get
# confused between strings and hashes for things like body and query
# parameters.
raise ArgumentError, "method should be a symbol" \
unless method.is_a?(Symbol)
raise ArgumentError, "uri should be a string" \
unless uri.is_a?(String)
raise ArgumentError, "body should be a string" \
if body && !body.is_a?(String)
raise ArgumentError, "headers should be a hash" \
if headers && !headers.is_a?(Hash)
raise ArgumentError, "query should be a string" \
if query && !query.is_a?(String)

connection = connection_for(uri)

u = URI.parse(uri)
path = if query
u.path + "?" + query
else
u.path
end

connection.send_request(method.to_s.upcase, path, body, headers)
end

#
# private
#

# `uri` should be a parsed `URI` object.
private def create_connection(uri)
# These all come back as `nil` if no proxy is configured.
proxy_host, proxy_port, proxy_user, proxy_pass = proxy_parts

connection = Net::HTTP.new(uri.host, uri.port,
proxy_host, proxy_port,
proxy_user, proxy_pass)

connection.open_timeout = Stripe.open_timeout
connection.read_timeout = Stripe.read_timeout

connection.use_ssl = uri.scheme == "https"

if Stripe.verify_ssl_certs
connection.verify_mode = OpenSSL::SSL::VERIFY_PEER
connection.cert_store = Stripe.ca_store
else
connection.verify_mode = OpenSSL::SSL::VERIFY_NONE

unless @verify_ssl_warned
@verify_ssl_warned = true
warn("WARNING: Running without SSL cert verification. " \
"You should never do this in production. " \
"Execute `Stripe.verify_ssl_certs = true` to enable " \
"verification.")
end
end

connection
end

# `Net::HTTP` somewhat awkwardly requires each component of a proxy URI
# (host, port, etc.) rather than the URI itself. This method simply parses
# out those pieces to make passing them into a new connection a little less
# ugly.
private def proxy_parts
if Stripe.proxy.nil?
[nil, nil, nil, nil]
else
u = URI.parse(Stripe.proxy)
[u.host, u.port, u.user, u.password]
end
end
end
end
131 changes: 131 additions & 0 deletions lib/stripe/multipart_encoder.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
# frozen_string_literal: true

require "securerandom"

module Stripe
# Encodes parameters into a `multipart/form-data` payload as described by RFC
# 2388:
#
# https://tools.ietf.org/html/rfc2388
#
# This is most useful for transferring file-like objects.
#
# Parameters should be added with `#encode`. When ready, use `#body` to get
# the encoded result and `#content_type` to get the value that should be
# placed in the `Content-Type` header of a subsequent request (which includes
# a boundary value).
class MultipartEncoder
MULTIPART_FORM_DATA = "multipart/form-data".freeze

# A shortcut for encoding a single set of parameters and finalizing a
# result.
#
# Returns an encoded body and the value that should be set in the content
# type header of a subsequent request.
def self.encode(params)
encoder = MultipartEncoder.new
encoder.encode(params)
encoder.close
[encoder.body, encoder.content_type]
end

# Gets the object's randomly generated boundary string.
attr_reader :boundary

# Initializes a new multipart encoder.
def initialize
# We seed this with an empty string so that it encoding defaults to UTF-8
# instead of ASCII. The empty string is UTF-8 and new string inherits the
# encoding of the string it's seeded with.
@body = String.new("")

# Chose the same number of random bytes that Go uses in its standard
# library implementation. Easily enough entropy to ensure that it won't
# be present in a file we're sending.
@boundary = SecureRandom.hex(30)

@closed = false
@first_field = true
end

# Gets the encoded body. `#close` must be called first.
def body
raise "object must be closed before getting body" unless @closed
@body
end

# Finalizes the object by writing the final boundary.
def close
raise "object already closed" if @closed

@body << "\r\n"
@body << "--#{@boundary}--"

@closed = true

nil
end

# Gets the value including boundary that should be put into a multipart
# request's `Content-Type`.
def content_type
"#{MULTIPART_FORM_DATA}; boundary=#{@boundary}"
end

# Encodes a set of parameters to the body.
#
# Note that parameters are expected to be a hash, but a "flat" hash such
# that complex substructures like hashes and arrays have already been
# appropriately Stripe-encoded. Pass a complex structure through
# `Util.flatten_params` first before handing it off to this method.
def encode(params)
raise "no more parameters can be written to closed object" if @closed

params.each do |name, val|
if val.is_a?(::File)
val.rewind
write_field(name, val.read, filename: ::File.basename(val.path))
elsif val.respond_to?(:read)
write_field(name, val.read, filename: "blob")
else
write_field(name, val, filename: nil)
end
end

nil
end

#
# private
#

# Escapes double quotes so that the given value can be used in a
# double-quoted string and replaces any linebreak characters with spaces.
private def escape(str)
str.gsub('"', "%22").tr("\n", " ").tr("\r", " ")
end

private def write_field(name, data, filename:)
if !@first_field
@body << "\r\n"
else
@first_field = false
end

@body << "--#{@boundary}\r\n"

if filename
@body << %(Content-Disposition: form-data) +
%(; name="#{escape(name.to_s)}") +
%(; filename="#{escape(filename)}"\r\n)
@body << %(Content-Type: application/octet-stream\r\n)
else
@body << %(Content-Disposition: form-data) +
%(; name="#{escape(name.to_s)}"\r\n)
end

@body << "\r\n"
@body << data.to_s
end
end
end
Loading

0 comments on commit 97db02f

Please sign in to comment.