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

URI.escape obsolete warning on Ruby 2.7+ #45

Open
nicolasrouanne opened this issue Jun 23, 2020 · 16 comments · May be fixed by #58
Open

URI.escape obsolete warning on Ruby 2.7+ #45

nicolasrouanne opened this issue Jun 23, 2020 · 16 comments · May be fixed by #58

Comments

@nicolasrouanne
Copy link

An annoying warning is raised by URI.encode (which is an alias of URI.escape) on Ruby 2.7+

docusign_esign-3.0.0/lib/docusign_esign/client/api_client.rb:262: warning: URI.escape is obsolete

Here is an article explaining why. TL;DR it's been obsolete for 10 years now but since Ruby 2.7, the warning is now displayed even when not running in verbose mode.
It's pretty annoying as it can quickly ruin one's logs.

As suggested per the stdlib doc, we could use CGI.escape instead.

This method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.

@nicolasrouanne
Copy link
Author

Note: it would not be a simple drop-in replacement.

CGI.escape is meant for encoding query params only, encodes spaces in the wrong format (+ instead %20). See this thread for all the available options. Not one size fits all, nor simple.

One option, would be to add the addressable gem. It adds a runtime dependency though.

> Addressable::URI.encode("https:://foo.com/bar?query=Programming Ruby:  The Pragmatic Programmer's Guide")
 => "https:://foo.com/bar?query=Programming%20Ruby:%20%20The%20Pragmatic%20Programmer's%20Guide" 

@LarryKlugerDS
Copy link
Contributor

Thank you for the bug report. I have submitted Jira DCM-4337 for our engineering team to evaluate the issue.

Thank you,
Larry

@nicolasrouanne
Copy link
Author

If the path to fix this is to add the addressable gem, I'll be happy to open a PR. If not, I'd rather let you guys fix this, as it's more tricky.

@emilford
Copy link

👋 @LarryKlugerDS has any progress been made on this? If not, I can take a look and open a PR in the coming days.

@sposin
Copy link

sposin commented Feb 4, 2021

Any update on having this resolved? We are working on an upgrade to ruby 3, but this is a blocker.

@sposin
Copy link

sposin commented Feb 5, 2021

Looks like the sendinblue gem has been updated. What is the status of Docusign therefore being updated?

@gsnavin
Copy link
Contributor

gsnavin commented Feb 11, 2021

We have released a new RC version with a fix, please give it a try and let us know if you see any issues. Thanks and sorry we couldn't get the fix out sooner.

V2.1 - https://rubygems.org/gems/docusign_esign/versions/3.8.0.rc1
V2 - https://rubygems.org/gems/docusign_esign/versions/2.8.0.rc1

@sposin
Copy link

sposin commented Feb 11, 2021

V2 - https://rubygems.org/gems/docusign_esign/versions/2.8.0.rc1 worked for me! Thank you!!!

@thibaultadet
Copy link

V2.1 - https://rubygems.org/gems/docusign_esign/versions/3.8.0.rc1 we still have the warnings
Screenshot 2021-02-15 at 14 28 51

@LyricL-Gitster
Copy link

LyricL-Gitster commented Mar 16, 2021

on Ruby 3, undefined method 'encode' for URI:Module

@barash-asenov
Copy link

barash-asenov commented Jun 3, 2021

version 2.9 works with Ruby3

@LyricL-Gitster
Copy link

LyricL-Gitster commented Jun 3, 2021

On 3.8.0.rc1 with Ruby 3.0.1 I added this file to my initializers as a workaround:

module DocuSign_eSign
  # Fix for obsolete URI methods.
  # See this issue for details: https://github.com/docusign/docusign-ruby-client/issues/45
  class Configuration
    def base_url
      "#{scheme}://#{[host, base_path].join('/').gsub(/\/+/, '/')}".sub(/\/+\z/, '')
    end
  end

  class ApiClient
    def build_request_url(path, opts)
      # Add leading and trailing slashes to path
      path = "/#{path}".gsub(/\/+/, '/')
      return "https://" + self.get_oauth_base_path + path if opts[:oauth]
      @config.base_url + path
    end
  end
end

@brettwgreen
Copy link

Did the RC fix for 3.8.0 not get retained in 3.10.0 or 3.10.0.RC? I tried both of those versions and still get the URI warning.

@brettwgreen
Copy link

Did the RC fix for 3.8.0 not get retained in 3.10.0 or 3.10.0.RC? I tried both of those versions and still get the URI warning.

I guess that fix never made it into main branch. I see the Open PR that has a test failure.

@hugovandevliert
Copy link

hugovandevliert commented Sep 8, 2021

Looks like this is fixed in the 3.12 release. No more warnings for me.

@brettwgreen
Copy link

Looks like this is fixed in the 3.12 release. No more warnings for me.

Agreed... upgraded and the warning is now gone!

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

Successfully merging a pull request may close this issue.

10 participants