Skip to content

Commit fbfc599

Browse files
committed
- Fix timed TCP connects for high POSIX file descriptors
Use poll() for POSIX TcpSocket timed connects instead of select(). select() cannot safely handle descriptor values >= FD_SETSIZE, so a process with many open sockets/resources could fail new HTTPS requests even when only one socket was being waited on. This showed up in the HTML compat module when loading request-heavy pages such as old Reddit. Keep the Windows path on select(), since Winsock does not use nfds as a POSIX descriptor limit and this code only waits on one socket. Also initialize HttpConnection::mIsKeepAlive in the TcpSocket-taking constructor, matching the default constructor and avoiding undefined connection lifetime state. Add a regression test that reserves file descriptors past FD_SETSIZE and verifies a timed localhost TcpSocket connect still succeeds. - Treat drawable ref "none" as clearing the layer drawable instead of trying to load it as a drawable/image reference. - Round the scroll view inner width up before deciding horizontal scrollbar visibility, avoiding fractional pixel overflow from creating a bogus horizontal scrollbar.
1 parent b7facf4 commit fbfc599

5 files changed

Lines changed: 101 additions & 25 deletions

File tree

src/eepp/network/http.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,8 +1059,7 @@ Http::Response Http::downloadRequest( const Http::Request& request, IOStream& wr
10591059
contentLength = 0;
10601060
}
10611061

1062-
if ( mConnection &&
1063-
received.getField( "connection" ) == "close" ) {
1062+
if ( mConnection && received.getField( "connection" ) == "close" ) {
10641063
mConnection->setConnected( false );
10651064
mConnection->setTunneled( false );
10661065
}
@@ -1618,7 +1617,11 @@ Http::HttpConnection::HttpConnection() :
16181617
mIsKeepAlive( false ) {}
16191618

16201619
Http::HttpConnection::HttpConnection( TcpSocket* socket ) :
1621-
mSocket( socket ), mIsConnected( false ), mIsTunneled( false ), mIsSSL( false ) {}
1620+
mSocket( socket ),
1621+
mIsConnected( false ),
1622+
mIsTunneled( false ),
1623+
mIsSSL( false ),
1624+
mIsKeepAlive( false ) {}
16221625

16231626
Http::HttpConnection::~HttpConnection() {
16241627
eeSAFE_DELETE( mSocket );

src/eepp/network/tcpsocket.cpp

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,15 @@
55
#include <eepp/network/tcpsocket.hpp>
66
#include <eepp/system/clock.hpp>
77
#include <eepp/system/log.hpp>
8-
8+
#include <limits>
9+
910
#if EE_PLATFORM == EE_PLATFORM_HAIKU
1011
#include <sys/select.h>
1112
#endif
13+
14+
#if EE_PLATFORM != EE_PLATFORM_WIN
15+
#include <poll.h>
16+
#endif
1217

1318
#ifdef _MSC_VER
1419
#pragma warning( disable \
@@ -127,30 +132,37 @@ Socket::Status TcpSocket::connect( const IpAddress& remoteAddress, unsigned shor
127132
if ( !blocking )
128133
return status;
129134

130-
// Otherwise, wait until something happens to our socket (success, timeout or error)
131-
if ( status == Socket::NotReady ) {
132-
#if EE_PLATFORM != EE_PLATFORM_WIN
133-
if ( getHandle() >= FD_SETSIZE ) {
134-
// The socket FD is too large for select().
135-
// You cannot safely use FD_SET.
136-
setBlocking( true );
137-
return Error;
138-
}
139-
#endif
140-
141-
// Setup the selector
142-
fd_set selector;
143-
FD_ZERO( &selector );
144-
FD_SET( getHandle(), &selector );
135+
// Otherwise, wait until something happens to our socket (success, timeout or error)
136+
if ( status == Socket::NotReady ) {
137+
#if EE_PLATFORM == EE_PLATFORM_WIN
138+
// Setup the selector
139+
fd_set selector;
140+
FD_ZERO( &selector );
141+
FD_SET( getHandle(), &selector );
145142

146143
// Setup the timeout
147144
timeval time;
148145
time.tv_sec = static_cast<long>( timeout.asMicroseconds() / 1000000 );
149146
time.tv_usec = static_cast<long>( timeout.asMicroseconds() % 1000000 );
150147

151-
// Wait for something to write on our socket (which means that the connection request
152-
// has returned)
153-
if ( select( static_cast<int>( getHandle() + 1 ), NULL, &selector, NULL, &time ) > 0 ) {
148+
// Wait for something to write on our socket (which means that the connection request
149+
// has returned)
150+
if ( select( static_cast<int>( getHandle() + 1 ), NULL, &selector, NULL, &time ) > 0 ) {
151+
#else
152+
pollfd descriptor;
153+
descriptor.fd = getHandle();
154+
descriptor.events = POLLOUT;
155+
descriptor.revents = 0;
156+
157+
Int64 timeoutMilliseconds = ( timeout.asMicroseconds() + 999 ) / 1000;
158+
int pollTimeout = timeoutMilliseconds > std::numeric_limits<int>::max()
159+
? std::numeric_limits<int>::max()
160+
: static_cast<int>( timeoutMilliseconds );
161+
162+
// Wait for something to write on our socket (which means that the connection request
163+
// has returned). poll() is not limited by FD_SETSIZE, unlike select().
164+
if ( poll( &descriptor, 1, pollTimeout ) > 0 ) {
165+
#endif
154166
// At this point the connection may have been either accepted or refused.
155167
// To know whether it's a success or a failure, we must check the address of the
156168
// connected peer

src/eepp/ui/uinodedrawable.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,11 @@ void UINodeDrawable::LayerDrawable::setDrawable( Drawable* drawable, const bool&
654654
}
655655

656656
void UINodeDrawable::LayerDrawable::setDrawable( const std::string& drawableRef ) {
657+
if ( drawableRef == "none" ) {
658+
setDrawable( nullptr, false );
659+
return;
660+
}
661+
657662
if ( loadRemoteDrawable( drawableRef ) ) {
658663
mDrawableRef = drawableRef;
659664
return;

src/eepp/ui/uiscrollview.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,10 @@ void UIScrollView::containerUpdate() {
219219
mHScroll->setVisible( false );
220220
mHScroll->setEnabled( false );
221221
} else {
222-
bool visible = mScrollView->getPixelsSize().getWidth() > getPixelsSize().getWidth() -
223-
getPixelsPadding().Left -
224-
getPixelsPadding().Right;
222+
auto scrollViewWidth = mScrollView->getPixelsSize().getWidth();
223+
auto meInnerWidth = std::ceil( getPixelsSize().getWidth() - getPixelsPadding().Left -
224+
getPixelsPadding().Right );
225+
auto visible = scrollViewWidth > meInnerWidth;
225226

226227
mHScroll->setVisible( visible );
227228
mHScroll->setEnabled( visible );

src/tests/unit_tests/http.cpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,36 @@
22

33
#include <atomic>
44
#include <thread>
5+
#include <vector>
56

67
#include <eepp/network/http.hpp>
78
#include <eepp/network/tcplistener.hpp>
89
#include <eepp/network/tcpsocket.hpp>
910

11+
#if EE_PLATFORM != EE_PLATFORM_WIN
12+
#include <fcntl.h>
13+
#include <sys/select.h>
14+
#include <unistd.h>
15+
#endif
16+
1017
using namespace EE;
1118
using namespace EE::Network;
1219

20+
#if EE_PLATFORM != EE_PLATFORM_WIN
21+
namespace {
22+
23+
struct FileDescriptorGuard {
24+
~FileDescriptorGuard() {
25+
for ( int fd : fds )
26+
::close( fd );
27+
}
28+
29+
std::vector<int> fds;
30+
};
31+
32+
} // namespace
33+
#endif
34+
1335
UTEST( Http, responseHeaderLineLargerThanReceiveBuffer ) {
1436
TcpListener listener;
1537
ASSERT_EQ( listener.listen( Socket::AnyPort, IpAddress::LocalHost ), Socket::Done );
@@ -55,3 +77,36 @@ UTEST( Http, responseHeaderLineLargerThanReceiveBuffer ) {
5577
EXPECT_EQ( response.getStatus(), Http::Response::Ok );
5678
EXPECT_TRUE( response.getBody() == "hello" );
5779
}
80+
81+
#if EE_PLATFORM != EE_PLATFORM_WIN
82+
UTEST( Http, tcpConnectTimeoutHandlesFdAboveFdSetSize ) {
83+
TcpListener listener;
84+
ASSERT_EQ( listener.listen( Socket::AnyPort, IpAddress::LocalHost ), Socket::Done );
85+
86+
FileDescriptorGuard openFiles;
87+
openFiles.fds.reserve( FD_SETSIZE + 16 );
88+
while ( openFiles.fds.empty() || openFiles.fds.back() < FD_SETSIZE ) {
89+
int fd = ::open( "/dev/null", O_RDONLY );
90+
if ( fd < 0 )
91+
break;
92+
openFiles.fds.push_back( fd );
93+
}
94+
95+
if ( openFiles.fds.empty() || openFiles.fds.back() < FD_SETSIZE )
96+
UTEST_SKIP( "could not reserve enough file descriptors" );
97+
98+
TcpSocket client;
99+
Socket::Status status =
100+
client.connect( IpAddress::LocalHost, listener.getLocalPort(), Seconds( 5 ) );
101+
EXPECT_EQ( status, Socket::Done );
102+
103+
if ( status == Socket::Done ) {
104+
TcpSocket accepted;
105+
EXPECT_EQ( listener.accept( accepted ), Socket::Done );
106+
accepted.disconnect();
107+
}
108+
109+
client.disconnect();
110+
listener.close();
111+
}
112+
#endif

0 commit comments

Comments
 (0)