Failing atob test case for malformed input

[cburgmer]

Upgrading org.seleniumhq.selenium:htmlunit3-driver to v4.28.0 fails a local project. The change set points to an upgrade of htmlunit from 4.7.0 to 4.9.0. Included in this change set is a switch (7a3c32b) to java.util.Base64. I suspect that switch comes with a subtle change in an edge case with invalid input.

This change adds a failing test case with an expectation for a InvalidCharacterError as documented in https://developer.mozilla.org/en-US/docs/Web/API/Window/atob.

I have not implemented a fix as to first confirm my understanding.

[rbri]

@cburgmer will have a look - but sounds reasonalble

[rbri]

@cburgmer will merge this, and fix it soon.

[rbri]

@cburgmer fix is in, please have a look and if you can think about other error situations please provide simply as comment here

Do you need a fresh snapshot build?

[cburgmer]

Thanks! I cannot think of other edge cases as of now.

I will have to wait for a new upstream version of the selenium driver, so no rush in releasing, it would likely not speed up things much.

[rbri]

Guten Morgen @cburgmer

after some sleet and a bit more thinking i fear that this fix will not solve your problem. From my understanding handling this error was also not correct in the old versions. Therefore i guess the behavior is not so much different from the old one.
What is really different is the 'forgiveness' of the commons codec impl. Some invalid stuff is simply ignored and as far as i know the java impl is more picky here.
Maybe you can do a check with the real stuff from your app under test and see if there is something more to fix.

Sonnige Grüße Ronald

[cburgmer]

If I use the bleeding edge build my script executes successfully, if I go back to the latest version if fails. Here's how I tweaked it to take the bleeding edge:

        <dependency>
            <groupId>org.seleniumhq.selenium</groupId>
            <artifactId>htmlunit3-driver</artifactId>
            <version>4.29.0</version>
            <exclusions>
              <exclusion>
                <artifactId>htmlunit</artifactId>
                <groupId>org.htmlunit</groupId>
              </exclusion>
            </exclusions>
        </dependency>
        <dependency>
            <artifactId>htmlunit</artifactId>
            <groupId>org.htmlunit</groupId>
            <version>4.11.0-SNAPSHOT</version>
        </dependency>

For me this proves the change works.

My assumption and oversimplified understanding also was the following:
With the current release the JS execution fails and stops the thread, while with the correct DOM error the script is able to recover and continue. Do let me know if this is incorrect.

[rbri]

looks like we are both correct :-D

Have done the same test with version 4.7.0. The code passes without throwing an error because the commons lib simply returns '' instead of throwing. Therefore your tests passing....

So the old impl had the same bug - not doing the right think at invalid input.
But with your new test we are compatible in this case. Thanks a lot...

[rbri]

and the whitespace handling was also broken...