CODEWITHSUNDEEP
ruby-on-railsruby
aRtoo
aRtoo

Reputation: 1892

Execute a dynamic method using eval or public_send

I have these code that executes a dynamic method. I'm using eval here to execute it but what I wanted to do is changed it to public_send because I was told so and it's much safer.

Current code:

  # update workstep logic here.
        incoming_status = params[params[:name]]

        # grab workflow, this is current data, use this to compare status to in comming status
        workflow = get_workorder_product_workstep(params[:workflow_id])

        # check current status if its pending allow to update
        # security concern EVAL!
       
        if eval("workflow.can_#{incoming_status}?")
          # update status
          eval("workflow.#{incoming_status}")
          # updated attribute handled_by
          workflow.update_attributes(handled_by_id: @curr_user.id)
          workflow.save
        else
          flash[:notice] = 'Action not allowed'
        end

The eval here is the concern. How can I changed this to public_send?

Here's what I did.

public_send("workflow.can_#{incoming_status}?")

public_send("#{workflow}.can_#{incoming_status}?")

both of them doesn't work. gives me an error of no method. The first public error returns this undefined method workflow.can_queue? for #<Spree::Admin::WorkordersController:0x00007ff71c8e6f00>

But it should work because I have a method workflow.can_queue?

the second error on public is this undefined method #<Spree::WorkorderProductWorkstep:0x00007ff765663550>.can_queue? for #<Spree::Admin::WorkordersController:0x00007ff76597f798>

I think for the second workflow is being evaluated separately? I'm not sure.

Upvotes: 0

Views: 522

Answers (1)

spickermann
spickermann

Reputation: 106782

Working with public_send you can change the relevant lines to:

if workflow.public_send("can_#{incoming_status}?")
  # update status
  workflow.public_send(incoming_status.to_s)
    # ...

A note about security and risks

workflow.public_send("can_#{xyz}?") can only call methods on workflow that are public and which start with the prefix can_ and end with ?. That is probably only a small number of methods and you can easily decide if you want to allow all those methods.

workflow.public_send("#{incoming_status'}) is different because it allows all public methods on workflow – even destroy. That means using this without the "can_#{incoming_status}?" is probably a bad idea. Or you should at least first check if incoming_status is in a whitelist of allowed methods.

eval is the worst because it will evaluate the whole string without any context (e.q. an object like workflow). Imaging you have eval("workflow.#{incoming_status}") without to check first if incoming_status is actually allowed. If someone then sends an incoming_status like this "to_s; system('xyz')"then xyz could be everything – like commands to send a hidden file via email, to install a backdoor or to delete some files.

Upvotes: 4

Related Questions