docusign / docusign-esign-ruby-client

The Official DocuSign Ruby Library used to interact with the eSign REST API. Send, sign, and approve documents using this client.
MIT License
66 stars 62 forks source link

URI.escape obsolete warning on Ruby 2.7+ #45

Open nicolasrouanne opened 4 years ago

nicolasrouanne commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

Thank you, Larry

nicolasrouanne commented 4 years ago

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 commented 4 years ago

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

sposin commented 3 years ago

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

sposin commented 3 years ago

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

gsnavin commented 3 years ago

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 commented 3 years ago

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

thibaultadet commented 3 years ago

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 commented 3 years ago

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

barash-asenov commented 3 years ago

version 2.9 works with Ruby3

LyricL-Gitster commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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

brettwgreen commented 2 years ago

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

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