ruffle icon indicating copy to clipboard operation
ruffle copied to clipboard

getURL has some issues depending on the backend

Open Toad06 opened this issue 5 years ago • 6 comments

getURL("https://ruffle.rs","_self","GET"); Sample: getURL.zip

Web

  • An extra character (?) is added to the destination URL.

  • (FIXED) The panic screen briefly appears when this code is executed. This is a regression. I've run a bisect, the issue seems to have been introduced in commit 739af3f8d71eaf5981686823fa312715f23608a1. The following error is logged into the console: panicked at 'cannot recursively acquire mutex', /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/sys/wasm/../unsupported/mutex.rs:22:9 (this issue doesn't happen if the target is _blank)

Desktop (Windows)

  • Lots of characters are added to the destination URL: ?%24version=LNX+32%2C0%2C0%2C0&instance2=_level0.instance2&instance1=_level0.instance1

  • The official Flash Player projector seems to ignore this code and doesn't open the URL.

Toad06 avatar Dec 25 '20 21:12 Toad06

On web, maybe we should move the shared object flush from beforeunload to unload so that it doesn't run immediately when navigation occurs.

Herschel avatar Dec 27 '20 11:12 Herschel

👍

I've tested that in Chrome and Firefox (Windows) and that did the trick: no more panic screen. This is also still good for #2046.

Toad06 avatar Dec 27 '20 17:12 Toad06

I had the similar issue, my case getURL("https://test.com?Correct=c","_self","GET");. But after it only redirects me to https://test.com?

amrography avatar Jan 10 '21 18:01 amrography

The official Flash Player projector seems to ignore this code and doesn't open the URL.

Yes, the Flash projector tends to ignore these kinds of commands when playing a local file, for security reasons I assume. Let's look at the behavior when loading this SWF via a web server to get a more accurate test. I am running python -m http.server and running the SWF via this URL: http://localhost:8000/getURL.swf.

When I load the above URL into the Flash projector or a Flash-enabled browser, it sends me to https://ruffle.rs/, as expected.

But when I load that into the Ruffle desktop app or web demo page, I get this URL instead: https://ruffle.rs/?%24version=LNX+32%2C0%2C0%2C0&instance2=_level0.instance2&instance1=_level0.instance1

My conclusion is that Ruffle's implementation of getURL must be incorrect. As seen in the AS2 language reference, the second argument to getURL specifies a window to load the URL into. It seems like Ruffle is trying to evaluate "_self" as an ActionScript variable instead of treating it as meaning "load into the current window".

Now let's look at the issue with query strings raised by @amrography. Here's a modified version of the getURL SWF that tries to navigate to https://ruffle.rs/?test=a: getURLq.zip

Loading http://localhost:8000/getURLq.swf into the Flash projector and a Flash-enabled browser and clicking again works as expected.

But strangely, this exposes a difference between desktop and web Ruffle builds. Desktop Ruffle now tries to navigate to this URL: https://ruffle.rs/?test=a&%24version=LNX+32%2C0%2C0%2C0&instance2=_level0.instance2&instance1=_level0.instance1 While the web demo seems to ignore the query string entirely and behave exactly the same as the original getURL.swf: https://ruffle.rs/?%24version=LNX+32%2C0%2C0%2C0&instance2=_level0.instance2&instance1=_level0.instance1

So here we have another problem - web builds of Ruffle are ignoring query strings passed to getURL. Hopefully this can be diagnosed and fixed too.

n0samu avatar Sep 24 '22 01:09 n0samu

So I have been looking into this and it's coming from activation.locals_into_form_values in avm1/globals/movie_clip.rs which leads to this function in activation.rs; where 3 specific local variables are being erroneously added and turned into strings; I added some debugging info see where the properties it is asking for come from which is below.

All lookups were performed on StageObjectData { base: ScriptObject(GcCell(Gc { ptr: 0x1209f642cf8 })), display_object: MovieClip(MovieClip(GcCell(Gc { ptr: 0x12093672708 }))), text_field_bindings: [TextFieldBinding { text_field: EditText(GcCell(Gc { ptr: 0x1209d7a71f8 })), variable_name: "off" }] } 

Found $version
- Represented as String("LNX 32,0,0,0") 
- Inside the base object which is ScriptObject(GcCell(Gc { ptr: 0x1209f642cf8 }))
----
Found instance2 
- Inside child display object, EditText(EditText(GcCell(Gc { ptr: 0x1209d7a71f8 })))
----
Found instance1
- Inside child display object, Avm1Button(Avm1Button(GcCell(Gc { ptr: 0x1209fa3dde8 })))

With the test swf's you provided it spits out the "correct" URL: https://ruffle.rs/?test=a&%24version=LNX+32%2C0%2C0%2C0&instance2=_level0.instance2&instance1=_level0.instance1

  • It turned instance... into _level0.instance... when it stringified the child display objects.

I am at a loss about what the specific bug is, as it seems the code is directly querying object properties and it is getting them. The fix might be additional checks on what properties are allowed to become form variables, but I am not well versed enough to say that wouldn't break other things.

golfinq avatar Oct 11 '22 02:10 golfinq

I'm seeing a recent regression that I was able to trace back to this.

For some reason, changing the page fragment in an about:srcdoc frame is adding this junk data, navigating to about:srcdoc?%24version=LNX+32%2C0%2C0%2C0&instanc…nstance1&instance19=_level0.instance19#audioStart, which breaks components that depend on that navigation.

GiovanH avatar Oct 08 '24 00:10 GiovanH