Menu

#1890 Wrong JS engine setup in case of modified browser version

2.27
closed
RBRi
None
1
2017-12-11
2017-06-07
No

If I use a cloned browser version with a modified user agent string, the JavaScriptEngine setup may no longer match the browser version from which the clone was made. See the attached test case to reproduce.

Looks like the issue lies in AbstractJavaScriptConfiguration.getClassConfiguration() where BrowserVersion.equals() is now used (instead of isChrome(), etc.) to determine the base browser type. Since the cloned and modified browser version is never equal to any of the predefined browser versions, we will always end up with a setup for EDGE in this case.

Discussion

  • Joerg Werner

    Joerg Werner - 2017-06-07

    Added test case.

     
  • Joerg Werner

    Joerg Werner - 2017-06-07

    I noticed that BrowserVersion objects are also used as key in hash maps. So messing around with the writable fields of BrowserVersion may lead to undesired effects, such as entries not found or creating additional entries for each modification. Not sure if resorting to the base browser type (nick) as key would be a solution.

     
  • RBRi

    RBRi - 2017-06-11
    • status: open --> accepted
    • assigned_to: RBRi
     
  • RBRi

    RBRi - 2017-06-11

    correct in all points. Have changed AbstractJavaScriptConfiguration like you suggested. Have to think more about the equals/hash think - suggestinons are welcome

     
  • Ahmed Ashour

    Ahmed Ashour - 2017-06-11

    I believe we have inconsisgency, because BrowserVersion can be constructed by the user, but that can't fully configure how it used (e.g. with @JsxGetter in 'host' classes). And even .isChrome() is not sufficient in those getters/setters/functions.

    Another concern is the performance of equals() if it is used in many places.

    What about removing the consturction, yet allowing the user to configure existing one (e.g. CHROME,FF52) with specific hearders, because I guess there are no user cases at the moment except for that.

     
    • RBRi

      RBRi - 2017-06-11

      Maybe we can ask our users if someone has a need for a more flexible solution via the mailing list.

       

      Last edit: RBRi 2017-07-25
  • Ahmed Ashour

    Ahmed Ashour - 2017-06-11

    Maybe we can ask our users if someone has a need for a more flexible solution via the mailing list.

    Sure, please go ahead and ask about the need to configure a BrowserVersion.

     
  • Joerg Werner

    Joerg Werner - 2017-06-13

    First of all, thanks for the quick fix, Ronald!

    With respect to BrowserVersion, I agree that probably no one will really need to customize the setup of a certain browser version except for the header configuration. It may even be advantageous to make BrowserVersion immutable (solves the issue of BrowserVersion as map key). But then the header customization must be possible elsewhere, for example in WebClientOptions.

    This is of course a breaking change and maybe there is another way. But whatever the solution will be, I believe that one thing is important: it should still be possible to have multiple web clients in the same JVM at the same time emulating the same browser type, but have different header configurations.

     
  • RBRi

    RBRi - 2017-07-24

    Next step was done, the (never really working) support for adding features during construction was removed and the nickname is now used for the dictionaries.

     
  • RBRi

    RBRi - 2017-12-11

    This is now part of our latest release.

     
  • RBRi

    RBRi - 2017-12-11
    • status: accepted --> closed
     

Log in to post a comment.

MongoDB Logo MongoDB