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

Lint/DeprecatedOpenSSLConstant suggestion with double quoted string #8035

Closed
tsugimoto opened this issue May 26, 2020 · 1 comment · Fixed by #8038 or #8040
Closed

Lint/DeprecatedOpenSSLConstant suggestion with double quoted string #8035

tsugimoto opened this issue May 26, 2020 · 1 comment · Fixed by #8038 or #8040
Labels

Comments

@tsugimoto
Copy link
Contributor

a.rb:2:3: W: Lint/DeprecatedOpenSSLConstant: Use OpenSSL::Cipher.new('AES-256-cbc') instead of OpenSSL::Cipher::AES256.new('cbc').
  OpenSSL::Cipher::AES256.new('cbc')
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
a.rb:5:3: W: Lint/DeprecatedOpenSSLConstant: Use OpenSSL::Cipher.new('AES-256-"cbc"') instead of OpenSSL::Cipher::AES256.new("cbc").
  OpenSSL::Cipher::AES256.new("cbc")
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The suggestion is valid with single quoted strings, but broken with double quoted ones.

Expected behavior

Suggestion should be OpenSSL::Cipher::AES256.new("AES-256-cbc") (preserving double quotes) at least, or one of the OpenSSL::Cipher.ciphers entries if we really want consistent casing.

Actual behavior

OpenSSL::Cipher.new('AES-256-"cbc"') does not exist.

Steps to reproduce the problem

do_something do
  OpenSSL::Cipher::AES256.new("cbc")
end

and

$ rubocop --only Lint/DeprecatedOpenSSLConstant

RuboCop version

$ rubocop -V
0.84.0 (using Parser 2.7.1.2, rubocop-ast 0.0.3, running on ruby 2.6.3 x86_64-darwin18)
@tsugimoto
Copy link
Contributor Author

And I've just found that it actually tries to concatenate non-strings:

FOO = "cbc"
do_something do
  OpenSSL::Cipher::AES256.new(FOO)
end
a.rb:3:3: W: Lint/DeprecatedOpenSSLConstant: Use OpenSSL::Cipher.new('AES-256-FOO') instead of OpenSSL::Cipher::AES256.new(FOO).
  OpenSSL::Cipher::AES256.new(FOO)
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@koic koic added the bug label May 26, 2020
koic added a commit to koic/rubocop that referenced this issue May 26, 2020
…nstant`

Fixes rubocop#8035.

This PR fixes the following false positive for `Lint/DeprecatedOpenSSLConstant`
when using double quoted string argument.

```console
% cat example.rb
OpenSSL::Cipher::AES256.new('cbc')
OpenSSL::Cipher::AES256.new("cbc")

% bundle exec rubocop --only Lint/DeprecatedOpenSSLConstant -a
(snip)

Offenses:

example.rb:1:1: W: [Corrected] Lint/DeprecatedOpenSSLConstant: Use
OpenSSL::Cipher.new('AES-256-cbc') instead of OpenSSL::Cipher::AES256.new('cbc').
OpenSSL::Cipher::AES256.new('cbc')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
example.rb:2:1: W: [Corrected] Lint/DeprecatedOpenSSLConstant: Use
OpenSSL::Cipher.new('AES-256-"cbc"') instead of OpenSSL::Cipher::AES256.new("cbc").
OpenSSL::Cipher::AES256.new("cbc")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

% cat example.rb
OpenSSL::Cipher.new('AES-256-cbc')
OpenSSL::Cipher.new('AES-256-"cbc"')
```

rubocop#8035 has another different false positive, which resolves as another PR.
bbatsov pushed a commit that referenced this issue May 26, 2020
Fixes #8035.

This PR fixes the following false positive for `Lint/DeprecatedOpenSSLConstant`
when using double quoted string argument.

```console
% cat example.rb
OpenSSL::Cipher::AES256.new('cbc')
OpenSSL::Cipher::AES256.new("cbc")

% bundle exec rubocop --only Lint/DeprecatedOpenSSLConstant -a
(snip)

Offenses:

example.rb:1:1: W: [Corrected] Lint/DeprecatedOpenSSLConstant: Use
OpenSSL::Cipher.new('AES-256-cbc') instead of OpenSSL::Cipher::AES256.new('cbc').
OpenSSL::Cipher::AES256.new('cbc')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
example.rb:2:1: W: [Corrected] Lint/DeprecatedOpenSSLConstant: Use
OpenSSL::Cipher.new('AES-256-"cbc"') instead of OpenSSL::Cipher::AES256.new("cbc").
OpenSSL::Cipher::AES256.new("cbc")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

% cat example.rb
OpenSSL::Cipher.new('AES-256-cbc')
OpenSSL::Cipher.new('AES-256-"cbc"')
```

#8035 has another different false positive, which resolves as another PR.
koic added a commit to koic/rubocop that referenced this issue May 26, 2020
…nstant`

Fixes rubocop#8035.

This PR fixes a false positive for `Lint/DeprecatedOpenSSLConstant`
when argument is a variable, method, or constant.

```console
% cat example.rb
MODE = 'cbc'
OpenSSL::Cipher::AES256.new(MODE)

% bundle exec rubocop --only Lint/DeprecatedOpenSSLConstant -a
(snip)

Offenses:

example.rb:2:1: W: [Corrected] Lint/DeprecatedOpenSSLConstant: Use
OpenSSL::Cipher.new('AES-256-MODE') instead of OpenSSL::Cipher::AES256.new(MODE).
OpenSSL::Cipher::AES256.new(MODE)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected

% cat example.rb
MODE = 'cbc'
OpenSSL::Cipher.new('AES-256-MODE')
```

RuboCop does not have a tracing feature a value of variable, (method,) and constant yet.
This PR accepts cases where these values are used to prevent auto-correction mistakes.

On the other hand, this change produces false negatives for the deprecated APIs.
If it is difficult to accept false negatives, I will update this PR to
warn against these cases and auto-correction will not performed.
bbatsov pushed a commit that referenced this issue May 31, 2020
Fixes #8035.

This PR fixes a false positive for `Lint/DeprecatedOpenSSLConstant`
when argument is a variable, method, or constant.

```console
% cat example.rb
MODE = 'cbc'
OpenSSL::Cipher::AES256.new(MODE)

% bundle exec rubocop --only Lint/DeprecatedOpenSSLConstant -a
(snip)

Offenses:

example.rb:2:1: W: [Corrected] Lint/DeprecatedOpenSSLConstant: Use
OpenSSL::Cipher.new('AES-256-MODE') instead of OpenSSL::Cipher::AES256.new(MODE).
OpenSSL::Cipher::AES256.new(MODE)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected

% cat example.rb
MODE = 'cbc'
OpenSSL::Cipher.new('AES-256-MODE')
```

RuboCop does not have a tracing feature a value of variable, (method,) and constant yet.
This PR accepts cases where these values are used to prevent auto-correction mistakes.

On the other hand, this change produces false negatives for the deprecated APIs.
If it is difficult to accept false negatives, I will update this PR to
warn against these cases and auto-correction will not performed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants