Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Net::HTTP::Persistent.write_timeout #586

Closed
wants to merge 104 commits into from

Conversation

maurycy
Copy link
Contributor

@maurycy maurycy commented Aug 8, 2021

Net::HTTP::Persistent.write_timeout now exposes Net::HTTP.write_timeout since drbrain/net-http-persistent#99

@flavorjones
Copy link
Member

@maurycy Thank you for submitting this! Looks good, but I'd like to make some time to do a deeper dive on the timeouts (the test coverage seems pretty thin for read_timeout and this might be a good opportunity for me to learn a bit more and improve the test coverage).

@maurycy
Copy link
Contributor Author

maurycy commented Aug 9, 2021

@flavorjones Thank you as well! That's true. I had to manually byebug.

@maurycy
Copy link
Contributor Author

maurycy commented Aug 9, 2021

@flavorjones While we're at it, what do you think about covering ssl_timeout as well? It's well supported by net-http-persistent. I can handle this.

Net::HTTP.keep_alive_timeout is shadowed by Net::HTTP::Persistent.idle_timeout, so not much we can do.

Net::HTTP.continue_timeout is not supported by net-http-persistent.

ncs1 and others added 11 commits November 9, 2021 06:48
…se-psych-safe-load

use safe_load when using Psych >= 3.1
…pdate-ci-to-ruby31

ci: update to cover Ruby 3.1
Note that in this case we treat cookies differently from credentials
per RFC 6265 section 8.5:

https://datatracker.ietf.org/doc/html/rfc6265#section-8.5

> Cookies do not provide isolation by port.  If a cookie is readable
> by a service running on one port, the cookie is also readable by a
> service running on another port of the same server.  If a cookie is
> writable by a service on one port, the cookie is also writable by a
> service running on another port of the same server.  For this
> reason, servers SHOULD NOT both run mutually distrusting services on
> different ports of the same host and use cookies to store security-
> sensitive information.
…edirect-headers

fix: clear credentials when redirecting to a different port
@flavorjones
Copy link
Member

@maurycy Since you emailed me directly let me respond here. Would you be willing to add test coverage to this PR that exercises the write timeout, either using the test server or via a mock? That would help me be confident in supporting it as a feature going forward.

flavorjones and others added 25 commits July 28, 2024 09:33
- move dev deps to the Gemfile
- brotli is not a dev dep for jruby (which it doesn't support)
- test how brotli responses are handled when Brotli is not loaded
It broke in CI where the TERM env var is not set, see minitest/minitest#1009
…ride-is-broken

test: don't use minitest/pride
Add Zstd support (optional dependency, just like Brotli)
mime-types 3.6.0 warns "MIME::Type.MIME::Type.new when called with a String is deprecated."

change introduced in and syntax borrowed from:
mime-types/ruby-mime-types@8512c15
to one that supports modern ruby.
fix: mime type string deprecation
We can add them back if they restabilize, but recently these tests
have been very very noisy and simultaneously they are low value.
…i-drop-jruby-and-truffleruby

ci: drop jruby and truffleruby, add 3.4
…arning-for-3-4

Fix frozen string warning for Ruby 3.4
by using URI::RFC23996_PARSER directly if it exists.
…uby-34-warnings

quash Ruby 3.4 URI::Parser warnings
@flavorjones
Copy link
Member

Github seems to have corrupted this branch, it claims there are 106 commits touching 69 files despite me having just rebased it. Going to close and re-open a new one with your commit.

@flavorjones
Copy link
Member

See #663

@flavorjones
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.