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

proposal: encoding/json: add a new method to decode json html escaped directly to struct #24408

Closed
johnbalvin opened this issue Mar 15, 2018 · 6 comments

Comments

@johnbalvin
Copy link
Contributor

johnbalvin commented Mar 15, 2018

Please answer these questions before submitting your issue. Thanks!

What did you do?

I wanted to decode json with its fields being html escaped

import (
    "html/template" 
    "encoding/json"
    "net/http"
    "io"
    "io/ioutil"
    "log"
)

func anyFunction(w http.ResponseWriter, r *http.Request) {
    body, err := ioutil.ReadAll(r.Body)
    if err != nil {
        log.Print(err)
    }
    ri, wo := io.Pipe()
    go template.HTMLEscape(wo, body)   
    var t []customStruct
    json.NewDecoder(ri).Decode(t) //error: Invalid character:'&' looking for beginning of object key string
    ...
}

What did you expect to see?

CustumStruct to be fill with the escape html

What did you see instead?

error: Invalid character:'&' looking for beginning of object key string

###More information
I asked on stackoverlow: https://stackoverflow.com/questions/49288607/scape-html-from-json-post-request-in-go but there is not actually an efficient solution to this issue
As far as I understand, with the tools provided right now there are 2 posibles solutions:
1-Creating a method for each struct (customMarshalJSON) as https://golang.org/pkg/encoding/json/ says so
2.-Escaping the html after unmarshal

Both of them solve the issue, but not in an efficient way, wouldn´t be more efficient scaping the html while decoding? like having a method as this:
json.UnmarshalHTMLEscape(data, &struct)

@gopherbot gopherbot added this to the Proposal milestone Mar 15, 2018
@mvdan
Copy link
Member

mvdan commented Mar 15, 2018

This seems wrong to me; JSON should have nothing to do with HTML escaping, nor with any other form of escaping or encoding that isn't JSON.

Also, have you benchmarked this? What are you worried about - unnecessary allocations, or your code performing noticeably worse than your expectations or some similar code in another language?

It seems like you want to make JSON decoding and HTML escaping faster. If you control the data being received, I would instead suggest looking at alternatives like protobuf, which are designed for performance. Trying to squeeze performance out of the standard library's JSON and HTML packages can only go so far.

@johnbalvin
Copy link
Contributor Author

johnbalvin commented Mar 15, 2018

It´s to avoid code repeating in order to make code more readable and performance, in order to perform what you are saying I would have to repeat the same code over and over again for each struct just for escaping html. In JSON package the is a https://golang.org/pkg/encoding/json/#HTMLEscape htmlscape option but for encoding, not for decoding

@mvdan
Copy link
Member

mvdan commented Mar 15, 2018

Your markdown urls are backwards - they are not clickable.

I didn't know about this HTML quirk. I thought what you were asking for was an option like Encoder.SetEscapeHTML, such as Decoder.SetUnescapeHTML. But, as one would expect, that works out of the box: https://play.golang.org/p/RPLjORgScjv

I still think this has nothing to do with JSON. Escaping HTML when encoding is useful - to make the encoded JSON bytes less prone to buggy decoders. But escaping HTML when decoding seems completely unrelated to that. Why exactly do you need it?

@johnbalvin
Copy link
Contributor Author

johnbalvin commented Mar 15, 2018

To avoid xss by first decoding to html scaped, so any input like <>

will be &#60;&#62; I know I can do it after, when using the data, but I think it would be much more efficient if all data gets verify first and then saved in databased, instead of always escaping when sending data

@johnbalvin
Copy link
Contributor Author

It would be a performance improvement when sending data because if I escape when data is coming and then save in database, I wouldn´t need to escape it when sending the data, so it would be faster, of course this is not like the end of the world, but suposse there is a large ammount request asking for the data, I would save resources and response timing if I don´t escape when sending the data

@dsnet dsnet changed the title proposal: add a new method to json package to decode json html escaped directly to struct proposal: encoding/json: add a new method to decode json html escaped directly to struct Mar 15, 2018
@bradfitz
Copy link
Contributor

bradfitz commented Apr 2, 2018

Let's move this to the mailing list for now (https://golang.org/wiki/Questions) until a concrete problem is identified. Also, the JSON package is currently frozen for a few cycles at which time a bunch of issues will be addressed in aggregate, so there's no rush on this bug.

@bradfitz bradfitz closed this as completed Apr 2, 2018
@golang golang locked and limited conversation to collaborators Apr 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants