From e75ad1de0f95f38b45748cafb1212560fe7587f5 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 6 Jul 2023 15:06:24 +0200 Subject: [PATCH] Merge pull request from GHSA-9pxv-6qvf-pjwc * Fix timeout handling of outbound HTTP requests * Use CLOCK_MONOTONIC instead of Time.now --- app/lib/request.rb | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/app/lib/request.rb b/app/lib/request.rb index 0508169dcb..c76ec6b642 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -7,11 +7,48 @@ require 'resolv' # Monkey-patch the HTTP.rb timeout class to avoid using a timeout block # around the Socket#open method, since we use our own timeout blocks inside # that method +# +# Also changes how the read timeout behaves so that it is cumulative (closer +# to HTTP::Timeout::Global, but still having distinct timeouts for other +# operation types) class HTTP::Timeout::PerOperation def connect(socket_class, host, port, nodelay = false) @socket = socket_class.open(host, port) @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay end + + # Reset deadline when the connection is re-used for different requests + def reset_counter + @deadline = nil + end + + # Read data from the socket + def readpartial(size, buffer = nil) + @deadline ||= Process.clock_gettime(Process::CLOCK_MONOTONIC) + @read_timeout + + timeout = false + loop do + result = @socket.read_nonblock(size, buffer, exception: false) + + return :eof if result.nil? + + remaining_time = @deadline - Process.clock_gettime(Process::CLOCK_MONOTONIC) + raise HTTP::TimeoutError, "Read timed out after #{@read_timeout} seconds" if timeout || remaining_time <= 0 + return result if result != :wait_readable + + # marking the socket for timeout. Why is this not being raised immediately? + # it seems there is some race-condition on the network level between calling + # #read_nonblock and #wait_readable, in which #read_nonblock signalizes waiting + # for reads, and when waiting for x seconds, it returns nil suddenly without completing + # the x seconds. In a normal case this would be a timeout on wait/read, but it can + # also mean that the socket has been closed by the server. Therefore we "mark" the + # socket for timeout and try to read more bytes. If it returns :eof, it's all good, no + # timeout. Else, the first timeout was a proper timeout. + # This hack has to be done because io/wait#wait_readable doesn't provide a value for when + # the socket is closed by the server, and HTTP::Parser doesn't provide the limit for the chunks. + timeout = true unless @socket.to_io.wait_readable(remaining_time) + end + end end class Request