Project

General

Profile

Bug #16500

Argument is added to both splat and last &block argument

Added by anatolik (Anatol Pomozov) about 1 month ago. Updated about 1 month ago.

Status:
Open
Priority:
Normal
Target version:
-
[ruby-core:96769]

Description

Here is a followup for a ruby2.7 issue discussed here https://gitlab.com/groups/gitlab-org/-/epics/2380

I run gitlab with ruby2.7. gitlab/lib/api/api_guard.rb calls Rack's use method:

use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request|
  # The authenticator only fetches the raw token string

  # Must yield access token to store it in the env
  request.access_token
end

The use method looks like:

def use(middleware, *args, &block)
  if @map
    mapping, @map = @map, nil
      @use << proc { |app| generate_map app, mapping }
  end
  @use << proc { |app| middleware.new(app, *args, &block) }
end

For some reason, a Proc object was set to &block and added to args. It sounds wrong. A Proc should only be set to &block, and args should contain only one argument.


Related issues

Related to Ruby master - Bug #16504: `foo(*args, &args.pop)` should pass all elements of argsOpenActions

Updated by mame (Yusuke Endoh) about 1 month ago

Thank you for the report! I cannot reproduce the issue by a simple config.ru:

require "rack/oauth2"

use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request|
  request.access_token
end

Adding p args into the definition of Rack's use shows only ["The API"]. So I'd like to try it with the source code of gitlab. I have no idea at all about gitlab, so could you tell me how to reproduce it?

Updated by ioquatix (Samuel Williams) about 1 month ago

We cannot reproduce this.

Can you make some script to reproduce this in isolation?

Including Gemfile and Gemfile.lock details.

Updated by ioquatix (Samuel Williams) about 1 month ago

  • Assignee set to ioquatix (Samuel Williams)
  • Status changed from Open to Rejected

Updated by ioquatix (Samuel Williams) about 1 month ago

  • Status changed from Rejected to Open

On Ruby 2.7.0:

irb(main):020:-> x = [1, 2, ->{}]; puts(*x, &x.pop)
1
2
=> nil
irb(main):021:-> x = [1, 2, ->{}]; puts(*x, &x.last)
1
2
#<Proc:0x0000562763a56398 (irb):21 (lambda)>
=> nil

This seems like buggy behaviour related to the original issue.

Updated by jeremyevans0 (Jeremy Evans) about 1 month ago

ioquatix (Samuel Williams) wrote:

On Ruby 2.7.0:

irb(main):020:-> x = [1, 2, ->{}]; puts(*x, &x.pop)
1
2
=> nil
irb(main):021:-> x = [1, 2, ->{}]; puts(*x, &x.last)
1
2
#<Proc:0x0000562763a56398 (irb):21 (lambda)>
=> nil

This seems like buggy behaviour related to the original issue.

I'm not sure if that behavior is buggy. puts ignores a passed block, so the behavior seems expected.

def a(*args, &b)
  p [args, b]
end
x = [1, 2, ->{}]; a(*x, &x.pop)
# => [[1, 2], #<Proc:0x00001100c1362518 (irb):4 (lambda)>]
x = [1, 2, ->{}]; a(*x, &x.last)
# => [[1, 2, #<Proc:0x000011004f2b0ba0 (irb):5 (lambda)>], #<Proc:0x000011004f2b0ba0 (irb):5 (lambda)>]

Same results with Ruby 1.9.

Updated by mame (Yusuke Endoh) about 1 month ago

Ruby 2.7 partially changed the behavior. Coped from my comment: https://github.com/ruby-grape/grape/issues/1967#issuecomment-573366122

# in 2.6 or before
args = [1, 2, -> {}]; foo(   *args, &args.pop) #=> passes [1, 2] (bug; [1, 2, ->{}] is expected)
args = [1, 2, -> {}]; foo(0, *args, &args.pop) #=> passes [0, 1, 2] (bug; [0, 1, 2, ->{}] is expected)

# in 2.7
args = [1, 2, -> {}]; foo(   *args, &args.pop) #=> passes [1, 2] (bug; [1, 2, ->{}] is expected)
args = [1, 2, -> {}]; foo(0, *args, &args.pop) #=> passes [0, 1, 2, ->{}] (good)

So, there are two issues.

#8

Updated by mame (Yusuke Endoh) about 1 month ago

  • Related to Bug #16504: `foo(*args, &args.pop)` should pass all elements of args added
#9

Updated by sawa (Tsuyoshi Sawada) about 1 month ago

  • Description updated (diff)
  • Subject changed from Argument added both to splat and last &block argument to Argument is added to both splat and last &block argument

Updated by Eregon (Benoit Daloze) about 1 month ago

mame (Yusuke Endoh) wrote:

  • The behavior of args = [1, 2, -> {}]; foo( *args, &args.pop) should pass [1, 2, ->{}], and we should fix the bug on the ruby interpreter.

Why is that better than the previous behavior?
Changing that will be more incompatible.

Updated by Eregon (Benoit Daloze) about 1 month ago

FWIW the 2.6 behavior is detailed in ruby/spec:
https://github.com/ruby/spec/blob/84d606aa8e85a8fef6521b3402dae612d04288c4/language/send_spec.rb#L424-L445
From commit
https://github.com/ruby/spec/commit/01992ab93dd893d9e8bf79db9f5ff7d250952097
which shows RubyGems used to rely on the 2.6 behavior.

What I want to say is existing code relies on 2.6 behavior.
It's probably good to always evaluate positional arguments before the block argument for consistency, but it's an incompatible change.

Also available in: Atom PDF