[Gnash-commit] /srv/bzr/gnash/trunk r10434: Update NetConnection comments and tests.

[Benjamin Wolsey]

------------------------------------------------------------
revno: 10434
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Mon 2008-12-15 10:41:52 +0100
message:
  Update NetConnection comments and tests.
modified:
  libcore/asobj/NetConnection_as.cpp
  testsuite/actionscript.all/NetConnection.as
  testsuite/gnashrc.in
    ------------------------------------------------------------
    revno: 10433.1.1
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Mon 2008-12-15 10:26:01 +0100
    message:
      Comments, make NetConnection test more accurate.
    modified:
      libcore/asobj/NetConnection_as.cpp
      testsuite/actionscript.all/NetConnection.as
      testsuite/gnashrc.in

=== modified file 'libcore/asobj/NetConnection_as.cpp'
--- a/libcore/asobj/NetConnection_as.cpp        2008-12-14 12:48:17 +0000
+++ b/libcore/asobj/NetConnection_as.cpp        2008-12-15 09:41:52 +0000
@@ -433,7 +433,8 @@
     {
         boost::intrusive_ptr<NetConnection_as> ptr = 
             ensureType<NetConnection_as>(fn.this_ptr);
-        // FIXME check if it's possible for the URL of a NetConnection to 
change between call()s
+        // FIXME check if it's possible for the URL of a 
+        // NetConnection to change between call()s
         ptr->_callQueue->tick();
         return as_value();
     };
@@ -550,6 +551,10 @@
 }
 
 
+/// FIXME: this should not use _uri, but rather take a URL argument.
+/// Validation should probably be done on connect() only and return a 
+/// bool indicating validity. That can be used to return a failure
+/// for invalid or blocked URLs.
 std::string
 NetConnection_as::validateURL() const
 {
@@ -633,6 +638,9 @@
 }
 
 
+/// Called on NetConnection.connect(null).
+//
+/// The status notification happens immediately, isConnected becomes true.
 void
 NetConnection_as::connect()
 {
@@ -643,23 +651,29 @@
 
 
 void
-NetConnection_as::connect(const std::string& uri)
+NetConnection_as::connect(const std::string& /*uri*/)
 {
 
-    // FIXME: RTMP URLs should attempt a connection (warning: this seems
-    // to be different for SWF8). Would probably return true on success and
-    // set isConnected.
+    // FIXME: We should attempt a connection here (this is called when an
+    // argument is passed to NetConnection.connect(url).
+    // Would probably return true on success and set isConnected.
     //
-    // For URLs starting with anything other than "rtmp://" no connection is
-    // initiated, but the uri is still set.
-    setURI(uri);
-
+    // Under certain circumstances, an an immediate failure notification
+    // happens. These are:
+    // a) sandbox restriction
+    // b) invalid URL? NetConnection.connect(5) fails straight away, but
+    //    could be either because a URL has to be absolute, perhaps including
+    //    a protocol, or because the load is attempted from the filesystem
+    //    and fails immediately.
+    // TODO: modify validateURL for doing this.
     _isConnected = false;
     _inError = true;
     notifyStatus(CONNECT_FAILED);
 }
 
 
+/// FIXME: This should close an active connection as well as setting the
+/// appropriate properties.
 void
 NetConnection_as::close()
 {
@@ -690,7 +704,11 @@
             hexify(buf.data(), buf.size(), false));
 #endif
 
-    // FIXME check that ptr->_uri is valid
+    // FIXME: Don't do this here. Use a single connection object member
+    // for all calls (depends on the following FIXME), and also check
+    // whether a connection exists and don't call() if it doesn't (can be
+    // done in the AS implementation to save processing arguments when
+    // not connected).
     URL url(validateURL());
 
     // FIXME check if it's possible for the URL of a NetConnection
@@ -948,9 +966,9 @@
 //
 /// For non-rtmp streams:
 //
-/// Returns undefined if there are no arguments. Otherwise true if the first
-/// argument is null, false if it is anything else. Undefined is also valid
-/// for SWF7 and above.
+/// Returns undefined if there are no arguments, true if the first
+/// argument is null, otherwise the result of the attempted connection.
+/// Undefined is also a valid argument for SWF7 and above.
 //
 /// The isConnected property is set to the result of connect().
 as_value
@@ -973,11 +991,13 @@
 
     const VM& vm = ptr->getVM();
     const std::string& uriStr = uri.to_string_versioned(vm.getSWFVersion());
+    
+    // This is always set without validification.
+    ptr->setURI(uriStr);
 
     // Check first arg for validity 
     if (uri.is_null() || (vm.getSWFVersion() > 6 && uri.is_undefined())) {
         ptr->connect();
-        ptr->setURI(uriStr);
     }
     else {
         if ( fn.nargs > 1 )

=== modified file 'testsuite/actionscript.all/NetConnection.as'
--- a/testsuite/actionscript.all/NetConnection.as       2008-12-03 21:22:15 
+0000
+++ b/testsuite/actionscript.all/NetConnection.as       2008-12-15 09:26:01 
+0000
@@ -152,7 +152,14 @@
 check_equals(result, "NetConnection.Connect.Success");
 check_equals(level, "status");
 
-ret = tmp.connect("http://someserver";);
+// The pp and Gnash sandboxes behave differently. The pp rejects any
+// network connection from filesystem-loaded SWFs unless the SWF location
+// is added to the player configuration file. This server is blacklisted
+// in the testsuite gnashrc file, so Gnash should refuse to load this too.
+// The test should work on both players for a SWF loaded from anywhere but
+// www.blacklistedserver.org (domain still available, in case anyone wants to
+// mess up the test).
+ret = tmp.connect("http://www.blacklistedserver.org";);
 check_equals(ret, false);
 check_equals(tmp.isConnected, false);
 check_equals(result, "NetConnection.Connect.Failed");

=== modified file 'testsuite/gnashrc.in'
--- a/testsuite/gnashrc.in      2008-10-25 09:06:46 +0000
+++ b/testsuite/gnashrc.in      2008-12-15 09:26:01 +0000
@@ -38,4 +38,7 @@
 #       so that test runners can check what's sent!
 set urlOpenerFormat echo '%u' > /dev/null
 
+# Blacklist (for testing NetConnection)
+set blacklist www.blacklistedserver.org
+
 # TODO: enable extensions ?