[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Gnash-commit] /srv/bzr/gnash/trunk r10878: Fix some bugs in XMLSocket i
|
From: |
Benjamin Wolsey |
|
Subject: |
[Gnash-commit] /srv/bzr/gnash/trunk r10878: Fix some bugs in XMLSocket implementation. |
|
Date: |
Mon, 18 May 2009 11:31:47 +0200 |
|
User-agent: |
Bazaar (1.13.1) |
------------------------------------------------------------
revno: 10878
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Mon 2009-05-18 11:31:47 +0200
message:
Fix some bugs in XMLSocket implementation.
modified:
libcore/asobj/XMLSocket_as.cpp
libnet/network.h
------------------------------------------------------------
revno: 10874.1.1
committer: Benjamin Wolsey <address@hidden>
branch nick: work
timestamp: Fri 2009-05-15 21:38:36 +0200
message:
Const correct Network::connected.
Add intermediate object between Network and XMLSocket_as to allow
not blocking AS execution during connection (not yet implemented).
Get the onConnect() order correct (still only correct in limited
cases).
Restore adding empty strings, as a non-automated test shows is the case.
modified:
libcore/asobj/XMLSocket_as.cpp
libnet/network.h
------------------------------------------------------------
revno: 10874.1.2
committer: Benjamin Wolsey <address@hidden>
branch nick: test
timestamp: Mon 2009-05-18 10:52:25 +0200
message:
Thread XMLSocket connection so that it doesn't block execution. As
expected,
onConnect is now called in the first advanceState() after the connection
attempt completes.
Port numbers above 65535 should be rejected immediately.
modified:
libcore/asobj/XMLSocket_as.cpp
=== modified file 'libcore/asobj/XMLSocket_as.cpp'
--- a/libcore/asobj/XMLSocket_as.cpp 2009-05-14 16:02:12 +0000
+++ b/libcore/asobj/XMLSocket_as.cpp 2009-05-18 08:52:25 +0000
@@ -1,4 +1,4 @@
-// xmlsocket.cpp: Network socket for XML-encoded information, for Gnash.
+// XMLSocket_as.cpp: Network socket for data (usually XML) transfer for Gnash.
//
// Copyright (C) 2005, 2006, 2007, 2008, 2009 Free Software Foundation, Inc.
//
@@ -33,27 +33,191 @@
#include "URLAccessManager.h"
#include "Object.h" // for getObjectInterface
#include "log.h"
+
+#include <boost/thread.hpp>
#include <boost/scoped_array.hpp>
#include <string>
-#define GNASH_XMLSOCKET_DEBUG
+#undef GNASH_XMLSOCKET_DEBUG
namespace gnash {
-static as_value xmlsocket_connect(const fn_call& fn);
-static as_value xmlsocket_send(const fn_call& fn);
-static as_value xmlsocket_new(const fn_call& fn);
-static as_value xmlsocket_close(const fn_call& fn);
-
-// These are the event handlers called for this object
-static as_value xmlsocket_onData(const fn_call& fn);
-
-static as_object* getXMLSocketInterface();
-static void attachXMLSocketInterface(as_object& o);
-static void attachXMLSocketProperties(as_object& o);
-
-
-class XMLSocket_as : public Network, public as_object {
+namespace {
+ as_value xmlsocket_connect(const fn_call& fn);
+ as_value xmlsocket_send(const fn_call& fn);
+ as_value xmlsocket_new(const fn_call& fn);
+ as_value xmlsocket_close(const fn_call& fn);
+
+ // These are the event handlers called for this object
+ as_value xmlsocket_onData(const fn_call& fn);
+
+ as_object* getXMLSocketInterface();
+ void attachXMLSocketInterface(as_object& o);
+}
+
+/// Connection object
+//
+/// A wrapper round a Network object that adds specific functions needed
+/// by XMLSocket.
+namespace {
+
+class SocketConnection
+{
+public:
+
+ SocketConnection()
+ :
+ _complete(false)
+ {}
+
+ /// Initiate a connection.
+ void connect(const std::string& host, boost::uint16_t port) {
+ _start = boost::thread(
+ boost::bind(&SocketConnection::makeConnection, this, host, port));
+ }
+
+ /// The state of the connection.
+ //
+ /// Until complete() is true, this may change.
+ bool connected() const {
+ return _socket.connected();
+ }
+
+ /// Whether an initiated connection is finished
+ //
+ /// @return true if a connection attempt is complete.
+ /// The connection attempt may have failed. Check
+ /// connected() to find out.
+ bool complete() const {
+ return _complete;
+ }
+
+ void setComplete() {
+ _complete = true;
+ }
+
+ size_t writeMessage(const std::string& str) {
+ // We have to write the null terminator as well.
+ return write(_socket.getFileFd(), str.c_str(), str.size() + 1);
+ }
+
+ /// Read from the socket.
+ void readMessages(std::vector<std::string>& msgs) {
+
+ assert(_socket.connected());
+
+ const int fd = _socket.getFileFd();
+ assert(fd > 0);
+
+ fd_set fdset;
+ struct timeval tval;
+ size_t retries = 10;
+
+ const int bufSize = 10000;
+ boost::scoped_array<char> buf(new char[bufSize]);
+
+ while (retries-- > 0) {
+ FD_ZERO(&fdset);
+ FD_SET(fd, &fdset);
+
+ tval.tv_sec = 0;
+ tval.tv_usec = 103;
+
+ const int ret = select(fd + 1, &fdset, NULL, NULL, &tval);
+
+ // If interupted by a system call, try again
+ if (ret == -1 && errno == EINTR) {
+ log_debug(_("The socket for fd #%d was interupted by a "
+ "system call"), fd);
+ continue;
+ }
+ if (ret == -1) {
+ log_error(_("XMLSocket: The socket for fd #%d was never "
+ "available"), fd);
+ return;
+ }
+
+ // Return if timed out.
+ if (ret == 0) return;
+
+ const size_t bytesRead = read(fd, buf.get(), bufSize - 1);
+
+ // Return if there's no data.
+ if (!bytesRead) return;
+
+ if (buf[bytesRead - 1] != 0) {
+ // We received a partial message, so bung
+ // a null-terminator on the end.
+ buf[bytesRead] = 0;
+ }
+
+ char* ptr = buf.get();
+ while (static_cast<size_t>(ptr - buf.get()) < bytesRead - 1) {
+
+#ifdef GNASH_XMLSOCKET_DEBUG
+ log_debug ("read: %d, this string ends: %d", bytesRead,
+ ptr + std::strlen(ptr) - buf.get());
+#endif
+
+ // If the string reaches to the final byte read, it's
+ // incomplete. Store it and continue. The buffer is
+ // NULL-terminated, so this cannot read past the end.
+ if (static_cast<size_t>(
+ ptr + std::strlen(ptr) - buf.get()) == bytesRead) {
+
+ _remainder += std::string(ptr);
+ break;
+ }
+
+ if (!_remainder.empty()) {
+ msgs.push_back(_remainder + std::string(ptr));
+ ptr += std::strlen(ptr) + 1;
+ _remainder.clear();
+ continue;
+ }
+
+ // Don't do anything if nothing is received.
+ msgs.push_back(ptr);
+
+ ptr += std::strlen(ptr) + 1;
+ }
+ }
+ }
+
+ /// Close the connection.
+ //
+ /// This also cancels any connection attempt in progress.
+ void close() {
+ _start.join();
+ _socket.closeNet();
+
+ // Reset for next connection.
+ _complete = false;
+
+ assert(_socket.getFileFd() <= 0);
+ assert(!_socket.connected());
+ }
+
+private:
+
+ void makeConnection(const std::string& host, boost::uint16_t port) {
+ _socket.createClient(host, port);
+ _complete = true;
+ }
+
+ Network _socket;
+
+ bool _complete;
+
+ std::string _remainder;
+
+ boost::thread _start;
+
+};
+
+}
+
+class XMLSocket_as : public as_object {
public:
@@ -62,165 +226,175 @@
XMLSocket_as();
~XMLSocket_as();
- bool connect(const std::string& host, short port);
-
- // Actionscript doesn't care about the result of either of these
- // operations.
+ /// True when the XMLSocket is not trying to connect.
+ //
+ /// If this is true but the socket is not connected, the connection
+ /// has failed.
+ bool ready() const {
+ return _ready;
+ }
+
+ /// Whether a connection exists.
+ //
+ /// This is not final until ready() is true.
+ bool connected() const {
+ return _connection.connected();
+ }
+
+ bool connect(const std::string& host, boost::uint16_t port);
+
+ /// Send a string with a null-terminator to the socket.
+ //
+ /// Actionscript doesn't care about the result.
void send(std::string str);
+
+ /// Close the socket
+ //
+ /// Actionscript doesn't care about the result.
void close();
-
- virtual void advanceState()
- {
- if (!_connected) return;
- checkForIncomingData();
- }
+ /// Called on advance() when socket is connected
+ virtual void advanceState();
private:
void checkForIncomingData();
- void fillMessageList(MessageList& msgs);
-
- /// Return the as_function with given name, converting case if needed
- boost::intrusive_ptr<as_function> getEventHandler(const std::string&
name);
-
- MessageList _messages;
-
- std::string _remainder;
+ /// Return the as_function with given name.
+ boost::intrusive_ptr<as_function> getEventHandler(
+ const std::string& name);
+
+ /// The connection
+ SocketConnection _connection;
+
+ bool _ready;
};
XMLSocket_as::XMLSocket_as()
:
- as_object(getXMLSocketInterface())
+ as_object(getXMLSocketInterface()),
+ _ready(false)
{
- attachXMLSocketProperties(*this);
}
+
XMLSocket_as::~XMLSocket_as()
{
- getVM().getRoot().removeAdvanceCallback(this);
-}
+ // Remove advance callback and close network connections.
+ close();
+}
+
+void
+XMLSocket_as::advanceState()
+{
+ // Wait until something has happened with the connection
+ if (!_connection.complete()) return;
+
+ // If this XMLSocket hadn't finished a connection, check whether it
+ // has now.
+ if (!ready()) {
+
+ if (!connected()) {
+
+ // If connection failed, notify onConnect and stop callback.
+ // This means advanceState() will not be called again until
+ // XMLSocket.connect() is invoked.
+ callMethod(NSV::PROP_ON_CONNECT, false);
+ _vm.getRoot().removeAdvanceCallback(this);
+ return;
+ }
+
+ // Connection succeeded.
+ callMethod(NSV::PROP_ON_CONNECT, true);
+ _ready = true;
+ }
+
+ // Now the connection is established we can receive data.
+ checkForIncomingData();
+}
+
bool
-XMLSocket_as::connect(const std::string& host, short port)
+XMLSocket_as::connect(const std::string& host, boost::uint16_t port)
{
- if ( ! URLAccessManager::allowXMLSocket(host, port) )
- {
+ if (!URLAccessManager::allowXMLSocket(host, port)) {
return false;
}
- bool success = createClient(host, port);
-
- assert( success || ! _connected );
-
- return success;
+ _connection.connect(host, port);
+
+ // Start callbacks on advance.
+ _vm.getRoot().addAdvanceCallback(this);
+
+ return true;
}
void
XMLSocket_as::close()
{
- assert(_connected);
-
- closeNet();
- // dunno why Network::closeNet() returns false always
- // doesn't make much sense to me...
- // Anyway, let's make sure we're clean
- assert(!_sockfd);
- assert(!_connected);
-}
-
+ _vm.getRoot().removeAdvanceCallback(this);
+ _connection.close();
+ _ready = false;
+}
+
+
+boost::intrusive_ptr<as_function>
+XMLSocket_as::getEventHandler(const std::string& name)
+{
+ boost::intrusive_ptr<as_function> ret;
+
+ as_value tmp;
+ string_table& st = getVM().getStringTable();
+ if (!get_member(st.find(name), &tmp) ) return ret;
+ ret = tmp.to_as_function();
+ return ret;
+}
void
-XMLSocket_as::fillMessageList(MessageList& msgs)
+XMLSocket_as::checkForIncomingData()
{
-
- const int fd = _sockfd;
+ assert(ready() && connected());
+
+ std::vector<std::string> msgs;
+ _connection.readMessages(msgs);
- if (fd <= 0) {
- log_error(_("XMLSocket: fd <= 0, returning false (timer not
unregistered "
- "while socket disconnected?"));
- return;
+ if (msgs.empty()) return;
+
+ log_debug(_("Got %d messages: "), msgs.size());
+
+#ifdef GNASH_XMLSOCKET_DEBUG
+ for (size_t i = 0, e = msgs.size(); i != e; ++i) {
+ log_debug(_(" Message %d: %s "), i, msgs[i]);
}
-
- fd_set fdset;
- struct timeval tval;
- size_t retries = 10;
-
- const int bufSize = 10000;
- boost::scoped_array<char> buf(new char[bufSize]);
-
- while (retries-- > 0) {
- FD_ZERO(&fdset);
- FD_SET(fd, &fdset);
-
- tval.tv_sec = 0;
- tval.tv_usec = 103;
-
- const int ret = select(fd + 1, &fdset, NULL, NULL, &tval);
-
- // If interupted by a system call, try again
- if (ret == -1 && errno == EINTR) {
- log_debug(_("The socket for fd #%d was interupted by a "
- "system call"), fd);
- continue;
- }
- if (ret == -1) {
- log_error(_("XMLSocket: The socket for fd #%d was never "
- "available"), fd);
- return;
- }
+#endif
+
+ as_environment env(_vm);
+
+ for (XMLSocket_as::MessageList::const_iterator it=msgs.begin(),
+ itEnd=msgs.end(); it != itEnd; ++it) {
- // Return if timed out.
- if (ret == 0) return;
-
- const size_t bytesRead = read(_sockfd, buf.get(), bufSize - 1);
-
- // Return if there's no data.
- if (!bytesRead) return;
-
- if (buf[bytesRead - 1] != 0)
- {
- // We received a partial message, so bung
- // a null-terminator on the end.
- buf[bytesRead] = 0;
- }
-
- char* ptr = buf.get();
- while (static_cast<size_t>(ptr - buf.get()) < bytesRead - 1)
- {
- log_debug ("read: %d, this string ends: %d",
- bytesRead, ptr + std::strlen(ptr) - buf.get());
- // If the string reaches to the final byte read, it's
- // incomplete. Store it and continue. The buffer is
- // NULL-terminated, so this cannot read past the end.
- if (static_cast<size_t>(
- ptr + std::strlen(ptr) - buf.get()) == bytesRead) {
- log_debug ("Setting remainder");
- _remainder += std::string(ptr);
- break;
- }
-
- if (!_remainder.empty())
- {
- log_debug ("Adding and clearing remainder");
- msgs.push_back(_remainder + std::string(ptr));
- ptr += std::strlen(ptr) + 1;
- _remainder.clear();
- continue;
- }
-
- // Don't do anything if nothing is received.
- if (std::strlen(ptr)) msgs.push_back(ptr);
-
- ptr += std::strlen(ptr) + 1;
- }
+ // This should be checked on every iteration in case one call
+ // changes the handler.
+ boost::intrusive_ptr<as_function> onDataHandler =
+ getEventHandler("onData");
+
+ if (!onDataHandler) break;
+
+ const std::string& s = *it;
+
+ std::auto_ptr<std::vector<as_value> > args(
+ new std::vector<as_value>);
+
+ args->push_back(s);
+ fn_call call(this, env, args);
+
+ onDataHandler->call(call);
}
-
+
}
@@ -229,21 +403,36 @@
void
XMLSocket_as::send(std::string str)
{
- if (!_connected)
- {
+ if (!ready() || !connected()) {
log_error(_("XMLSocket.send(): socket not initialized"));
- assert(_sockfd <= 0);
return;
}
- // We have to write the NULL terminator as well.
- int ret = write(_sockfd, str.c_str(), str.size() + 1);
+ _connection.writeMessage(str);
- log_debug(_("XMLSocket.send(): sent %d bytes, data was %s"), ret, str);
return;
}
+// extern (used by Global.cpp)
+void
+xmlsocket_class_init(as_object& global)
+{
+ // This is the global XMLSocket class
+ static boost::intrusive_ptr<builtin_function> cl;
+
+ if (!cl) {
+ cl = new builtin_function(&xmlsocket_new, getXMLSocketInterface());
+ }
+
+ // Register _global.XMLSocket
+ global.init_member("XMLSocket", cl.get());
+
+}
+
+
+namespace {
+
// XMLSocket.connect() returns true if the initial connection was
// successful, false if no connection was established.
as_value
@@ -259,41 +448,35 @@
boost::intrusive_ptr<XMLSocket_as> ptr =
ensureType<XMLSocket_as>(fn.this_ptr);
- if (ptr->connected())
- {
+ if (ptr->ready()) {
log_error(_("XMLSocket.connect() called while already "
"connected, ignored"));
+ return as_value(false);
}
as_value hostval = fn.arg(0);
const std::string& host = hostval.to_string();
- int port = int(fn.arg(1).to_number());
-
- if (!ptr->connect(host, port))
- {
- // TODO: onConnect(false) should not be called here, but rather
- // only if a failure occurs after the initial connection.
+ const double port = fn.arg(1).to_number();
+
+ // Port numbers above 65535 are rejected always, but not port numbers below
+ // 0. It's not clear what happens with them.
+ // TODO: find out.
+ // Other ports and hosts are checked against security policy before
+ // acceptance or rejection.
+ if (port > std::numeric_limits<boost::uint16_t>::max()) {
+ return as_value(false);
+ }
+
+ // XMLSocket.connect() returns false only if the connection is
+ // forbidden. The result of the real connection attempt is
+ // notified via onConnect().
+ const bool ret = ptr->connect(host, port);
+
+ if (!ret) {
log_error(_("XMLSocket.connect(): connection failed"));
- return as_value(false);
}
- // Actually, if first-stage connection was successful, we
- // should NOT invoke onConnect(true) here, but postpone
- // that event call to a second-stage connection checking,
- // to be done in a separate thread. The visible effect to
- // confirm this is that onConnect is invoked *after*
- // XMLSocket.connect() returned in these cases.
- // The same applies to onConnect(false), which will never
- // be called at the moment.
- //
- log_debug(_("XMLSocket.connect(): trying to call onConnect"));
- ptr->callMethod(NSV::PROP_ON_CONNECT, true);
-
- ptr->getVM().getRoot().addAdvanceCallback(ptr.get());
-
- log_debug(_("Timer set"));
-
- return as_value(true);
+ return as_value(ret);
}
@@ -303,8 +486,6 @@
as_value
xmlsocket_send(const fn_call& fn)
{
- GNASH_REPORT_FUNCTION;
-
boost::intrusive_ptr<XMLSocket_as> ptr =
ensureType<XMLSocket_as>(fn.this_ptr);
@@ -325,28 +506,15 @@
boost::intrusive_ptr<XMLSocket_as> ptr =
ensureType<XMLSocket_as>(fn.this_ptr);
- // If we're not connected, there's nothing to do
- if (!ptr->connected()) return as_value();
-
ptr->close();
return as_value();
}
as_value
-xmlsocket_new(const fn_call& fn)
+xmlsocket_new(const fn_call& /*fn*/)
{
boost::intrusive_ptr<as_object> xmlsock_obj = new XMLSocket_as;
-
-#ifdef GNASH_XMLSOCKET_DEBUG
- std::stringstream ss;
- fn.dump_args(ss);
- log_debug(_("new XMLSocket(%s) called - created object at "
- "%p"), ss.str(), static_cast<void*>(xmlsock_obj.get()));
-#else
- UNUSED(fn);
-#endif
-
return as_value(xmlsock_obj);
}
@@ -358,20 +526,20 @@
boost::intrusive_ptr<XMLSocket_as> ptr =
ensureType<XMLSocket_as>(fn.this_ptr);
- if (fn.nargs < 1)
- {
+ if (!fn.nargs) {
IF_VERBOSE_ASCODING_ERRORS(
- log_aserror(_("Builtin XMLSocket.onData() needs an argument"));
+ log_aserror(_("Builtin XMLSocket.onData() needs an argument"));
);
return as_value();
}
const std::string& xmlin = fn.arg(0).to_string();
+#ifdef GNASH_XMLSOCKET_DEBUG
log_debug("Arg: %s, val: %s", xmlin, fn.arg(0));
+#endif
- if (xmlin.empty())
- {
+ if (xmlin.empty()) {
log_error(_("Builtin XMLSocket.onData() called with an argument "
"that resolves to an empty string: %s"), fn.arg(0));
return as_value();
@@ -383,22 +551,20 @@
ptr->callMethod(NSV::PROP_ON_XML, arg);
return as_value();
-
}
-static as_object*
+as_object*
getXMLSocketInterface()
{
static boost::intrusive_ptr<as_object> o;
- if ( o == NULL )
- {
+ if (!o) {
o = new as_object(getObjectInterface());
attachXMLSocketInterface(*o);
}
return o.get();
}
-static void
+void
attachXMLSocketInterface(as_object& o)
{
o.init_member("connect", new builtin_function(xmlsocket_connect));
@@ -414,87 +580,8 @@
onDataIface->init_member(NSV::PROP_CONSTRUCTOR, onDataFun);
}
-void
-attachXMLSocketProperties(as_object& /*o*/)
-{
- // Has no own properties
-}
-
-// extern (used by Global.cpp)
-void xmlsocket_class_init(as_object& global)
-{
- // This is the global XMLSocket class
- static boost::intrusive_ptr<builtin_function> cl;
-
- if ( cl == NULL )
- {
- cl=new builtin_function(&xmlsocket_new, getXMLSocketInterface());
- // Do not replicate all interface to class!
- }
-
- // Register _global.String
- global.init_member("XMLSocket", cl.get());
-
-}
-
-boost::intrusive_ptr<as_function>
-XMLSocket_as::getEventHandler(const std::string& name)
-{
- boost::intrusive_ptr<as_function> ret;
-
- as_value tmp;
- string_table& st = getVM().getStringTable();
- if (!get_member(st.find(name), &tmp) ) return ret;
- ret = tmp.to_as_function();
- return ret;
-}
-
-void
-XMLSocket_as::checkForIncomingData()
-{
- assert(_connected);
-
- std::vector<std::string> msgs;
- fillMessageList(msgs);
-
- if (msgs.empty()) return;
-
- log_debug(_("Got %d messages: "), msgs.size());
-
-#ifdef GNASH_DEBUG
- for (size_t i = 0, e = msgs.size(); i != e; ++i)
- {
- log_debug(_(" Message %d: %s "), i, msgs[i]);
- }
-#endif
-
- as_environment env(_vm);
-
- for (XMLSocket_as::MessageList::const_iterator it=msgs.begin(),
- itEnd=msgs.end(); it != itEnd; ++it) {
-
- // This should be checked on every iteration in case one call
- // changes the handler.
- boost::intrusive_ptr<as_function> onDataHandler =
- getEventHandler("onData");
-
- if (!onDataHandler) break;
-
- const std::string& s = *it;
-
- std::auto_ptr<std::vector<as_value> > args(
- new std::vector<as_value>);
-
- args->push_back(s);
-
- fn_call call(this, env, args);
-
- onDataHandler->call(call);
- }
-
-}
-
-} // end of gnash namespace
+} // anonymous namespace
+} // gnash namespace
// Local Variables:
// mode: C++
=== modified file 'libnet/network.h'
--- a/libnet/network.h 2009-04-20 22:30:53 +0000
+++ b/libnet/network.h 2009-05-15 19:38:36 +0000
@@ -221,7 +221,7 @@
bool send(const char *str);
// Accessors for testing
- bool connected()
+ bool connected() const
{
assert ( ( _connected && _sockfd > 0 ) || ( ! _connected && _sockfd <=
0 ) );
return _connected;
- [Gnash-commit] /srv/bzr/gnash/trunk r10878: Fix some bugs in XMLSocket implementation.,
Benjamin Wolsey <=