Embedded single quote in UI::WebDialog callback block parameters causes SyntaxError exception

DanRathbun commented 6 years ago

SketchUp API Issue

Bug Reports

SketchUp Version: 2016 (likely later versions also.)
OS Platform: Windows

Description:

Sending a String with an embedded apostrophe (ie, a single quote character) causes a Ruby SyntaxError exception in the UI::WebDialog's internal passing of parameters to callback blocks.

This was a difficult one to "nail down" as the error appeared to be coming from my callback block's code, but is actually happening internally by the API handling of parameters whilst (or just before) passing them into my callback block(s).

I also had issue when I replaced the single quote with an embedded "'" character entity reference, but I cannot get this test example below to cause that same error. (I ended up using String.replace() to substitute a single quote with "???" and then revert that on the Ruby-side.)

Expectation:

It was expected that when using the JavaScript escape() function on the JS-side and then the CGI::unescape() method on the Ruby-side, we would not have any problems with special characters.

dlg = UI::WebDialog.new("Test",true,nil,436,160,250,250)

def dlg.test=(arg=true)
  @test = arg ? true : false
end

def dlg.test?
  @test
end

dlg.add_action_callback("accept") {|context,params|
  if dlg.test?
    puts "dialog context : "<<context.inspect
    puts "dialog params  : "<<params.inspect
  end
}

dlg.set_html(%[
    <!DOCTYPE html>
    <html>
    <head>
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <meta http-equiv="X-UA-Compatible" content="IE=11"/>
      <meta http-equiv="MSThemeCompatible" content="yes"/>
    </head>
    <body>

        <div id="dialog" width="100%">
            <textarea id="mytext" onfocus="this.select()" cols="50" rows="4"/>
            Here is some text that has an apostrophe denoting possesion. It is not the user's fault that this exception occurs when the text to sent back to Ruby.</textarea>
        </div>
        <p class="footer">
            <button onclick="accept();">Accept</button>
        </p>

        <script>

            var send_to_ruby = function (callback,param) {
                window.location = "skp:"+callback+"@"+escape(param);
            }

            var accept = function () {
                var param = document.getElementById("mytext").innerText;
                send_to_ruby( 'accept', param );
            }

        </script>

    </body>
    </html>
])

Now when you switch off @test before showing the dialog, you'll see that the error occurs before the block runs. If you switch @test back on, you'll see that the block DOES run in spite of the error, but the parameter is nil because of the error.

thomthom commented 6 years ago

Hmm... this might be an evaluation issue, similar to what we see with Sketchup.read_default.

I found that if you replace ' with \' then the string comes over without error:

dlg = UI::WebDialog.new("Test",true,nil,436,160,250,250)

def dlg.test=(arg=true)
  @test = arg ? true : false
end

def dlg.test?
  @test
end

dlg.add_action_callback("accept") {|context,params|
  if dlg.test?
    puts "dialog context : "<<context.inspect
    puts "dialog params  : "<<params.inspect
  end
}

dlg.set_html(%[
    <!DOCTYPE html>
    <html>
    <head>
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <meta http-equiv="X-UA-Compatible" content="IE=11"/>
      <meta http-equiv="MSThemeCompatible" content="yes"/>
    </head>
    <body>

        <div id="dialog" width="100%">
            <textarea id="mytext" cols="50" rows="4"/>
            It's a test's.</textarea>
        </div>
        <p class="footer">
            <button onclick="accept();">Accept</button>
        </p>

        <script>

            var send_to_ruby = function (callback,param) {
                var escaped_param = param.replace(/'/g, "\\\\'");
                window.location = "skp:"+callback+"@"+encodeURI(escaped_param);
            }

            var accept = function () {
                var param = document.getElementById("mytext").innerText;
                send_to_ruby( 'accept', param );
            }

        </script>

    </body>
    </html>
])

There's a bunch of issues with passing parameters via the skp protocol. I've resulted to setting the params in a hidden textarea and using get_element_value to fetch the params for a given callbacks. That avoids these URI encoding issues, the URI size limit etc.

Note, I also swapped out escape with encodeURI. MDN warns against the former. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/escape https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURI

DanRathbun commented 6 years ago

@thomthom Hmm... this might be an evaluation issue, similar to what we see with Sketchup.read_default.

Yes, this was also my determination finally.

Note, I also swapped out escape with encodeURI. MDN warns against the former.

Good to know generally. But it doesn't help in this case as the single quote (apostrophe) is one of the characters that encodeURI() does not encode.

So can CGI::unescape() handle strings created from encodeURI() alright ?

I found that if you replace ' with \' then the string comes over without error:

Weirdly elegant in it's simplicity. I didn't think to add another special character that might need to be itself escaped. (rolleyes)

But I see you also had to double escape ...

var escaped_param = param.replace(/'/g, "\\\\'");

Please add these tidbits of wisdom to the docs.

thomthom commented 6 years ago

Yea, the double escape is because first it needs escaping for Ruby to parse it - "Turning \\\\ into \\. Then \\ is sent to JavaScript to evaluate - which turns \\ into \.

As for encodeURI - I didn't notice it didn't escape ' (I found that surprising).

So can CGI::unescape() handle strings created from encodeURI() alright ?

Haven't tested. I would expect it should in theory. But I find URL escaping function in various languages PHP,JS,Ruby,Python can have subtle differences. (Though, maybe this have improved. You'd think standardization should dictate a uniform encoding.)

Please add these tidbits of wisdom to the docs.

I think it'd be worth adding a whole page by itself for the topic of WebDialog. And add references to that in the class documentation.

DanRathbun commented 6 years ago

As for encodeURI - I didn't notice it didn't escape ' (I found that surprising).

I did not say this. I had used escape(), which DID convert it to a %code, but I believe failed in the WebDialog parameter handling as well.

So this is why I tried ' and was also surprised that this failed as well. This led me to believe that the parameter handling code must be unescaping in C++ and then evaluating the results in Ruby.

I find it poor practice to hide the fact of internal evaluation from the documentation. We are all programmers here. The API dev purist will argue "implementation details are commonly withheld" but we have a need to know in this case (and the "default" methods) because we are feeding "the mill" and getting unexpected results. IF we know how "the mill" is grinding the corn, then we won't be surprised (or bitten) by results.

taustin73 commented 5 years ago

Logged SU-41965

SketchUp / api-issue-tracker

Embedded single quote in UI::WebDialog callback block parameters causes SyntaxError exception #113

SketchUp API Issue

Bug Reports