https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112017-03-28T22:52:39ZRuby Issue Tracking SystemRuby master - Feature #13379: [PATCH] safe IMAP connectionshttps://bugs.ruby-lang.org/issues/13379?journal_id=639312017-03-28T22:52:39Zahorek (Pavel Rosický)
<ul><li><strong>Subject</strong> changed from <i>Safe IMAP connections</i> to <i>[PATCH] safe IMAP connections</i></li></ul> Ruby master - Feature #13379: [PATCH] safe IMAP connectionshttps://bugs.ruby-lang.org/issues/13379?journal_id=639332017-03-29T02:33:15Zshugo (Shugo Maeda)
<ul><li><strong>Assignee</strong> set to <i>shugo (Shugo Maeda)</i></li></ul><p>ahorek (Pavel Rosický) wrote:</p>
<blockquote>
<p>Hi,<br>
I found out that using the standard IMAP library isn't very safe. It can be forced to hang the whole application.</p>
<p>the problem is here</p>
<pre><code>s = @sock.gets(CRLF)
</code></pre>
<p>-> the server accepted the connection but it didn't send any data. Now I need to reboot the server because my thread is blocked forever.</p>
<p>I have no other option but to use this</p>
<pre><code>Timeout.timeout(timeout, Net::OpenTimeout) { Net::IMAP.new(host, port, ssl) }
</code></pre>
<p>which basically works, but I really don't want to create a new thread for each IMAP call, so I did these changes:<br>
1/ replaced TCPSocket with Socket.tcp<br>
2/ replaced sock.read and sock.gets with sock.read_nonblock</p>
<p>now it works as expected.</p>
<p>Patch<br>
<a href="https://github.com/ruby/ruby/pull/1557" class="external">https://github.com/ruby/ruby/pull/1557</a></p>
</blockquote>
<p>Thanks for the patch, but it doesn't seem enough because:</p>
<ol>
<li>The :connect_timeout option of Socket.tcp doesn't work when DNS lookups by getaddrinfo block.<br>
A new thread by Timeout.timeout is needed to handle this case.</li>
<li>TLS handshakes in start_tls_session may also block. Protocol#ssl_socket_connect can be used to<br>
handle this case.</li>
</ol>
<p>And I'm not sure whether open_timeout should have the default value because a new thread is required.</p> Ruby master - Feature #13379: [PATCH] safe IMAP connectionshttps://bugs.ruby-lang.org/issues/13379?journal_id=639922017-03-29T20:44:44Zahorek (Pavel Rosický)
<ul><li><strong>File</strong> <a href="/attachments/6467">0001-ssl_socket_connect-for-imap.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/6467/0001-ssl_socket_connect-for-imap.patch">0001-ssl_socket_connect-for-imap.patch</a> added</li><li><strong>File</strong> <a href="/attachments/6466">0002-raise-Net-OpenTimeout.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/6466/0002-raise-Net-OpenTimeout.patch">0002-raise-Net-OpenTimeout.patch</a> added</li></ul><p>Thanks for the review. I've fixed the second case.</p>
<p>The only blocker is getaddrinfo now. It's a lowlevel system api that always blocks. I'll take a look if I can rewrite the ruby api to use getaddrinfo_a or getaddrinfo_ex (no thread is required). It'll be in a separate request.</p> Ruby master - Feature #13379: [PATCH] safe IMAP connectionshttps://bugs.ruby-lang.org/issues/13379?journal_id=639942017-03-30T02:53:00Zshugo (Shugo Maeda)
<ul></ul><p>ahorek (Pavel Rosický) wrote:</p>
<blockquote>
<p>Thanks for the review. I've fixed the second case.</p>
</blockquote>
<p>Thanks. The additional patches look fine.</p>
<blockquote>
<p>The only blocker is getaddrinfo now. It's a lowlevel system api that always blocks. I'll take a look if I can rewrite the ruby api to use getaddrinfo_a or getaddrinfo_ex (no thread is required). It'll be in a separate request.</p>
</blockquote>
<p>getaddrinfo_a() is glibc specific, so we need alternatives on other platforms such as FreeBSD.<br>
Do you mean GetAddrInfoEx() of winsock by <code>getaddrinfo_ex</code>?</p> Ruby master - Feature #13379: [PATCH] safe IMAP connectionshttps://bugs.ruby-lang.org/issues/13379?journal_id=640062017-03-30T15:21:59Zahorek (Pavel Rosický)
<ul></ul><p>Yes, these APIs are very platform specific. I was just checking documentations and existing solutions. Maybe I'll use the Resolv api, but I need to check incompatibilities and also a performance impact.</p> Ruby master - Feature #13379: [PATCH] safe IMAP connectionshttps://bugs.ruby-lang.org/issues/13379?journal_id=640072017-03-30T17:21:41Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p><a href="mailto:pdahorek@seznam.cz" class="email">pdahorek@seznam.cz</a> wrote:</p>
<blockquote>
<p>Yes, these APIs are very platform specific. I was just<br>
checking documentations and existing solutions. Maybe I'll use<br>
the Resolv api, but I need to check incompatibilities and also<br>
a performance impact.</p>
</blockquote>
<p>Yes, I actually prefer we use resolv more since it offers proper<br>
timeout support whereas getaddrinfo does not.</p>
<p>Thanks.</p> Ruby master - Feature #13379: [PATCH] safe IMAP connectionshttps://bugs.ruby-lang.org/issues/13379?journal_id=645542017-04-28T13:45:42Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li></ul> Ruby master - Feature #13379: [PATCH] safe IMAP connectionshttps://bugs.ruby-lang.org/issues/13379?journal_id=646502017-05-03T11:32:28Zshugo (Shugo Maeda)
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>Applied in changeset trunk|r58549.</p>
<hr>
<p>net/imap: handle timeouts</p>
<p>Patch by Pavel Rosický. [Feature <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: [PATCH] safe IMAP connections (Closed)" href="https://bugs.ruby-lang.org/issues/13379">#13379</a>] <a href="/issues/13379">[ruby-core:80440]</a></p> Ruby master - Feature #13379: [PATCH] safe IMAP connectionshttps://bugs.ruby-lang.org/issues/13379?journal_id=646512017-05-03T11:35:01Zshugo (Shugo Maeda)
<ul></ul><p>I've merged your patch. Thank you.</p>
<p>Create another ticket for resolv if it's ready.</p>