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
Allow accessing struct fields using lower case names #141
Comments
In Go, struct fields can't "not exist". All fields in your struct exist and contain their type's zero value, but since you declared them to be pointer types, their zero value is Do you mean differentiating between an |
@sauerbraten Yes, but it appears as if Jet explicitly gives an "does not exist in this scope" if the value is nil. If its not doing that then let me know and maybe there is a mistake on my part I'm overlooking. If that is the behavior then it looks like Line 270 in 11c8098
|
So my test case is: vars.Set("x_tag", nil) And then in my template I do: {{if x_tag}} Then I get the error: main.go:262: Jet Runtime Error ("x.jet":19): identifier "x_tag" is not available in the current scope map It looks like it should be accepted here: eval.go line 1102 is definitely where the error message is being emitted. It looks like x_tag is in the scope, but it has an invalid value which is why it can't be found. It is definitely becoming invalid in template.go line 367. So question is how do I correctly detect and add a nil value. I'm not very experienced with the reflect package but this works, so the issue seems to be a lack of type: Doing some Googling it looks like this a common issue when you start to mess with reflect, but I don't see a really good way to resolve it beyond what I did above. |
This is different from the example that you provide, isn't it? You're setting an untyped nil directly on the varmap. Should be able to do My opinion based on data to hand is that if you want to set untyped nils on the varmap and then check them, this is an odd use case, and you should just do
Can you give a concrete example of why this is valuable? |
@tooolbox There are two places where I think that being able to set the field names would be useful 1) Method and variables in title/pascal case might tip off an attacker that golang is the underlying system, using all lower case or camel case would be more consistent with javascript styles 2) the consistent use of lowercase is known to give a slight bump in compressibility because so much of html/javascript/css tends to also be written in lower case. |
|
@sauerbraten I'm thinking more in terms of giving third parties the ability to modify the templates - but without necessarily being aware of the underlying implementation. Right now this looks like a subset of handlebars so nodejs would probably be the assumption. |
@sauerbraten took the words out of my mouth.
Sounds pretty wild. Thoughts:
|
I'm inclined to say protecting against the attack vectors you describe (where an attacker has write access to your templates) is out of scope for Jet itself and you would have to address them in a layer you build around it, i.e. custom template validation. I renamed the issue to be about the lower case field access and will close it. We can discuss the "not in scope" vs. "set but nil" handling of nil pointer fields in a new issue if you want. |
@sauerbraten Turns out to be a fairly simple change, this will use the "json" name over the go field name if specified. Could just as easily be tagged "jet" or something custom.
|
That's not why I and @tooolbox are against this "feature". It's about keeping things simple. The feature you suggest could in fact be confusing to users. For example: type User struct {
Name string `json:-`
APIName string `json:"Name"`
} Imagine I have the above struct as part of my data model, and want to output lists of users on a website and via a JSON API. I use the JSON tag to make sure the JSON response gets the APIName as the user's name, and I use The behavior you suggest would have to be documented and would still be something users have to watch out for while giving them very little benefit (in most cases, none). We should be aiming to make Jet's behavior more predictable, unsurprising and intuitive. Sorry, but this change would do the opposite. |
@annismckenzie Curious your thoughts about this - internally my structs have a lot of *string, *int, *bool, etc. where the values can be nil if not set explicitly.
In Jet if a structure has a field and its value is nil then it appears to be the same as it not existing -- https://github.com/CloudyKit/jet/blob/master/eval.go#L1111
From a templating perspective I'd think they would be treated as safe values such as "", 0, false, etc. if it exists unless I'm explicitly comparing them with nil with isset. If I made a change to differentiate between a field that doesn't exist and a field that does exist but it set to nil, is that a change you would be interested in?
I was also thinking that using field tags to have a preferred name for struct fields (like encoding/json does) so that its not so obvious that Golang is involved in rendering the template or to obfuscate where the value might be coming from. It looks like there is just couple places in eval.go where a change would be needed to implement that. Would you be interested in that change if I got it working?
The text was updated successfully, but these errors were encountered: