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

Implement unmarchaling #2

Closed
Fale opened this issue Jun 5, 2017 · 9 comments
Closed

Implement unmarchaling #2

Fale opened this issue Jun 5, 2017 · 9 comments

Comments

@Fale
Copy link
Contributor

Fale commented Jun 5, 2017

I think it would be very useful to also implement unmarshaling functions to be able to secure also writes for post/put/patch.

@mweibel
Copy link
Collaborator

mweibel commented Jun 6, 2017

Can you explain what benefit that would bring over standard Golang JSON Unmarshal?

@Fale
Copy link
Contributor Author

Fale commented Jun 6, 2017

Let's pick this case: Using a PATCH update the fields that are passed (and that the user is authorized).

Trying to solve this, I did 2 different attempts:

Double json unmarshaling

Decode the POST data to Object with json/unmarshal, Filter the object with sheriff/marshal, marshal the map with json/marshal, unmarshal again with json/unmarshal.
The problem here is that - aside from the inefficiency - the fields that are not passed get blanked by the process.

mapstructure

The other method I came out with is:
Decode the POST data to Object with json/unmarshal, Filter the object with sheriff/marshal, convert the map to object with mapstructure.
This is more efficient, but present the same problem.

I think this scenario is pretty common and I think that an unmarshaling function could help here :)

@mweibel
Copy link
Collaborator

mweibel commented Jun 6, 2017

Hm.. I don't quite get why you do it that complicated?
Let's assume you have a post object as follows:

{
  "id": "12-23-53",
  "title": "Some random post title",
  "text": "Some text",
  "created_by": "23-64-99"
}

You have a struct representing this:

type Post struct {
  Id     string `json:"id"`
  Title string `json:"title"`
  Text string `json:"text"`
  CreatedBy string `json:"text" since:"2"`
}

Now you want to PATCH that Post in a Request:

PATCH /posts/12/12-23-53

{
  "title": "Foobar"
}

Why can't you just use normal json.Unmarshal into the Post struct?

@Fale
Copy link
Contributor Author

Fale commented Jun 6, 2017

The problem is that in that way I can not protect - for instance - the CreatedBy field, which I want to have it in "read-only mode". If I just json.Unmarshal the request, the user may change it

@mweibel
Copy link
Collaborator

mweibel commented Jun 6, 2017

But you anyway need to check what's in the update and potentially reply with a bad request in case the request contains things you don't want to be updated.

@Fale
Copy link
Contributor Author

Fale commented Jun 6, 2017

Yes I do, but I was trying to create a more generic code, adding some "publicupdate", "privateupdate", etc groups to semplify the code in the function because having 30+ fields and many different access levels, the code becomes very clunky otherwise

@mweibel
Copy link
Collaborator

mweibel commented Jun 14, 2017

But what would you want the lib to do when more data is passed than "allowed"? IMO sheriff is not a validation library, which is probably what you rather want (and maybe exists?).. no?

@Fale
Copy link
Contributor Author

Fale commented Jun 17, 2017

Yeah, I ended up implementing a couple of methods that I can share based on sheriff and reflection to actually do validation as well :).

Thanks for the great library!

@mweibel
Copy link
Collaborator

mweibel commented Jun 19, 2017

ok, let me know in case you published it somewhere :)

@mweibel mweibel closed this as completed Jun 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants