Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rb] Resolve uri gem deprecation warning #14770

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

Earlopain
Copy link
Contributor

@Earlopain Earlopain commented Nov 18, 2024

User description

Description

Ruby switches the default parser from RFC2396 to RFC3986. This parser contains all methods from the old parser but delegates to RFC2396 for a few and warns. Namely extract, make_regexp, escape, and unescape.

selenium currently supports Ruby >= 3.1, so the current old compatibility code is unnecessary. RFC2396_PARSER is a somewhat new addition, so some kind of check is still needed. It is available in Ruby 3.1 but early patch versions are missing it.

Motivation and Context

/home/user/.rbenv/versions/3.3.6/lib/ruby/gems/3.3.0/gems/selenium-webdriver-4.26.0/lib/selenium/webdriver/remote/bridge.rb:679: warning: URI::RFC3986_PARSER.escape is obsoleted. Use URI::RFC2396_PARSER.escape explicitly.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix


Description

  • Updated the escaper method in bridge.rb to resolve a deprecation warning by using URI::RFC2396_PARSER when available.
  • Ensured compatibility with Ruby versions >= 3.1 by checking for the presence of URI::RFC2396_PARSER.
  • This change addresses the obsolescence of URI::RFC3986_PARSER.escape.

Changes walkthrough 📝

Relevant files
Bug fix
bridge.rb
Resolve deprecation warning by updating URI parser usage 

rb/lib/selenium/webdriver/remote/bridge.rb

  • Updated the escaper method to use URI::RFC2396_PARSER if defined.
  • Fallback to URI::DEFAULT_PARSER if URI::RFC2396_PARSER is not defined.

  • +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    > /home/user/.rbenv/versions/3.3.6/lib/ruby/gems/3.3.0/gems/selenium-webdriver-4.26.0/lib/selenium/webdriver/remote/bridge.rb:679: warning: URI::RFC3986_PARSER.escape is obsoleted. Use URI::RFC2396_PARSER.escape explicitly.
    
    Ruby switches the default parser from RFC2396 to RFC3986. This parser contains
    all methods from the old parser but delegates to RFC2396 for a few and warns.
    Namely `extract`, `make_regexp`, `escape`, and `unescape`.
    
    selenium currently supports Ruby >= 3.1, so the current old compatibility code is unnecessary.
    `RFC2396_PARSER` is a somewhat new addition, so some kind of check is still needed.
    It is available in Ruby 3.1 but early patch versions are missing it.
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Backward Compatibility
    Verify that the fallback to URI::DEFAULT_PARSER works correctly on older Ruby versions where URI::RFC2396_PARSER is not defined

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add defensive programming to handle cases where neither URI parser is available

    Add a nil check for the escaper instance to handle potential undefined parser cases,
    preventing runtime errors.

    rb/lib/selenium/webdriver/remote/bridge.rb [689]

    -@escaper ||= defined?(URI::RFC2396_PARSER) ? URI::RFC2396_PARSER : URI::DEFAULT_PARSER
    +@escaper ||= if defined?(URI::RFC2396_PARSER)
    +  URI::RFC2396_PARSER
    +elsif defined?(URI::DEFAULT_PARSER)
    +  URI::DEFAULT_PARSER
    +else
    +  raise RuntimeError, 'No URI parser available'
    +end
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion adds important error handling to prevent silent failures if both URI parsers are undefined, which could lead to runtime errors. This defensive programming approach improves the code's robustness and provides clearer error messages.

    8

    💡 Need additional feedback ? start a PR chat

    Copy link
    Contributor

    @aguspe aguspe left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Documentation reference for this change: https://bugs.ruby-lang.org/issues/19266

    @aguspe aguspe merged commit 751bacb into SeleniumHQ:trunk Nov 19, 2024
    23 checks passed
    @aguspe
    Copy link
    Contributor

    aguspe commented Nov 19, 2024

    Thank you @Earlopain for your contribution! Have a great day!

    @Earlopain Earlopain deleted the uri-1.0-deprecation-warning branch November 19, 2024 20:06
    @Earlopain
    Copy link
    Contributor Author

    Thanks (: You have a good one as well

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants