6

I have a situation where people consuming our API will need to do a partial update in my resource. I understand that the HTTP clearly specifies that this is a PATCH operation, even though people on our side are used to send a PUT request for this and that's how the legacy code is built.

For exemplification, imagine the simple following struct:

type Person struct {
    Name string
    Age int
    Address string
}

On a POST request, I will provide a payload with all three values (Name, Age, Address) and validate them accordingly on my Golang backend. Simple.

On a PUT/PATCH request though, we know that, for instance, a name never changes. But say I would like to change the age, then I would simply send a JSON payload containing the new age:

PUT /person/1 {age:30}

Now to my real question: What is the best practice to prevent name from being used/updated intentionally or unintentionally modified in case a consumer of our API send a JSON payload containing the name field?

Example:

PUT /person/1 {name:"New Name", age:35} 

Possible solutions I thought of, but I don't actually like them, are:

  1. On my validator method, I would either forcibly remove the unwanted field name OR respond with an error message saying that name is not allowed.

  2. Create a DTO object/struct that would be pretty much an extension of my Person struct and then unmarshall my JSON payload into it, for instance

    type PersonPut struct { Age int Address string }

In my opinion this would add needless extra code and logic to abstract the problem, however I don't see any other elegant solution.

I honestly don't like those two approaches and I would like to know if you guys faced the same problem and how you solved it.

Thanks!

1
  • you can prevent the name field being copied into struct by using json "-" tag, but that will also prevent you from using the same struct for response in case your response has name field. The question then is, where exactly do you wan to control this struct. If you want to do it at the level of the handler, you could send back a bad request response in case current name is not the same as that in the struct. If you simple want to prevent an update, you could remove it right before calling the update service.
    – whitespace
    Commented Jan 10, 2021 at 7:37

5 Answers 5

1

The first solution your brought is a good one. Some well known frameworks use to implement similar logic.

As an example, latests Rails versions come with a built in solution to prevent users to add extra data in the request, causing the server to update wrong fields in database. It is a kind of whitelist implemented by ActionController::Parameters class.

Let's suppose we have a controller class as bellow. For purpose of this explanation, it contains two update actions. But you won't see it in real code.

class PeopleController < ActionController::Base

  # 1st version - Unsafe, it will rise an exception. Don't do it
  def update
    person = current_account.people.find(params[:id])
    person.update!(params[:person])
    redirect_to person
  end

  # 2nd version - Updates only permitted parameters
  def update
    person = current_account.people.find(params[:id])
    person.update!(person_params) # call to person_params method
    redirect_to person
  end


  private

  def person_params
    params.require(:person).permit(:name, :age)
  end

end

Since the second version allows only permitted values, it'll block the user to change the payload and send a JSON containing a new password value:

{ name: "acme", age: 25, password: 'account-hacked' }

For more details, see Rails docs: Action Controller Overview and ActionController::Parameters

2
  • What is the default behavior of .permit() in case I DO send name? Error message or silent ignore the field? Commented Jan 10, 2015 at 1:20
  • You can follow the strategy that best fit for you. In Rails, for example, there is no default whitelist, everything is blocked until you specify allowed fields. You can also always access field by field, but can't access the whole params object
    – jlucasps
    Commented Jan 13, 2015 at 1:12
1

If the name cannot be written it is not valid to provide it for any update request. I would reject the request if the name was present. If I wanted to be more lenient, I might consider only rejecting the request if name is different from the current name.

I would not silently ignore a name which was different from the current name.

1

This can be solved by decoding the JSON body into a map[string]json.RawMessage first. The json.RawMessage type is useful for delaying the actual decoding. Afterwards, a whitelist can be applied on the map[string]json.RawMessage map, ignoring unwanted properties and only decoding the json.RawMessages of the properties we want to keep.

The process of decoding the whitelisted JSON body into a struct can be automated using the reflect package; an example implementation can be found here.

0

I am not proficient on Golang but I believe a good strategy would be converting your name field to be a read-only field.

For instance, in a strictly object-oriented language as Java/.NET/C++ you can just provide a Getter but not a Setter.

Maybe there is some accessor configuration for Golang just like Ruby has....

If it is read-only then it shouldn't bother with receiving a spare value, it should just ignore it. But again, not sure if Golang supports it.

1
  • 2
    This is incorrect: the Name field is not "name" and Go does not let you have read-only fields. Also, Go does not normally need getters and setters for Json marshalling.
    – Rick-777
    Commented Jan 10, 2015 at 9:47
0

I think the clean way is to put this logic inside the PATCH handler. There should be some logic that would update only the fields that you want. Is easier if you unpack into a map[string]string and only iterate over the fields that you want to update. Additionally you could decode the json into a map, delete all the fields that you don't want to be updated, re-encode in json and then decode into your struct.

1
  • As I mentioned on the problem description I understand that this is a PATCH method, but we are not changing the logic from PUT so we won't affect people involved. Your method of iterating over all fields is not really needed in my case since I could simple write p.Name= "" and force it to an empty value. But, if I have to do this, why even allow consumers to send name in the first place? Commented Jan 10, 2015 at 1:24

Not the answer you're looking for? Browse other questions tagged or ask your own question.