search

evristzam / ndt

Automatically exported from code.google.com/p/ndt
Other
0 stars 0 forks source link

NDTUtils inadvertently overrides ServerHostname from FlashVars #111

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. Embed the applet and include a ServerHostname in FlashVars:
<param name="FlashVars" value="ServerHostname=oops.measurementlab.net">
2. Do not include a getNDTServer javascript function
3. Run a test

What is the expected output? What do you see instead?
Expected output is that the test will attempt to connect to 
oops.measurementlab.net. The actual output is that it attempts to connect to 
the default server from the applet.

The applet does not check to see if getNDTServer is defined before it attempts 
to invoke it. When the function is not defined in javascript, the 
ExternalInterface.call() excepts and our handler resets Main.server_hostname to 
the default. Please see the attached patch to fix this problem.

Original issue reported on code.google.com by hawki...@opentechinstitute.org on 7 Feb 2014 at 6:20

Attachments:

GoogleCodeExporter commented 9 years ago 
According to the AS documentation 
(http://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/flash/extern
al/ExternalInterface.html#call()), if the JS function is not defined, call() 
returns 'null'.
Have you tried to use that?

Original comment by tizi...@google.com on 10 Feb 2014 at 7:17

GoogleCodeExporter commented 9 years ago 
I have tried that, and it works a-okay, but now I see your point:

You are saying that if the check fails (and returns null but does NOT except), 
then the server_hostname will not be set? 

This did not come up in my test since I always set a ServerHostname using 
FlashVars. I will submit another patch presently.

Original comment by hawki...@measurementlab.net on 10 Feb 2014 at 8:33

GoogleCodeExporter commented 9 years ago 
On second thought (and inspection) this case does not particularly matter. The 
default server name is set before initializeFromHTML is run which means that 
when getNDTServerDefined is null falling through is actually a reasonable 
behavior. 

In fact, that makes the catch block to reset the server_hostname redundant too, 
actually. 

Does that make sense?

Original comment by hawki...@measurementlab.net on 10 Feb 2014 at 8:40

GoogleCodeExporter commented 9 years ago 
And of course now I parsed your comment correctly. You are saying that instead 
of calling an external function to check that the function exists, I can just 
check for a null return value. 

I will try that and repost a patch!

Will

Original comment by hawki...@measurementlab.net on 10 Feb 2014 at 10:18

GoogleCodeExporter commented 9 years ago 
Try this one on for size!

Original comment by hawki...@measurementlab.net on 10 Feb 2014 at 10:29

Attachments:

GoogleCodeExporter commented 9 years ago 
LGTM

Original comment by tizi...@google.com on 11 Feb 2014 at 7:59

GoogleCodeExporter commented 9 years ago 
Should I merge this or should you?

Original comment by hawki...@measurementlab.net on 11 Feb 2014 at 6:48

GoogleCodeExporter commented 9 years ago 
Change available in Issue111 -- T. says it's okay. Just give me the word and I 
can merge it.

Original comment by hawki...@measurementlab.net on 11 Feb 2014 at 6:58

GoogleCodeExporter commented 9 years ago 
Heard no complaints after the positive review from T. Merged. Closed.

Original comment by hawki...@measurementlab.net on 14 Feb 2014 at 4:57

GoogleCodeExporter commented 9 years ago

Original comment by skost...@soldevelo.com on 23 Jun 2014 at 5:52