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

text/template: interprets nil interface{} as a map #15356

Closed
walken-google opened this issue Apr 18, 2016 · 11 comments
Closed

text/template: interprets nil interface{} as a map #15356

walken-google opened this issue Apr 18, 2016 · 11 comments
Assignees
Milestone

Comments

@walken-google
Copy link
Contributor

@walken-google walken-google commented Apr 18, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    1.6.1
  2. What operating system and processor architecture are you using (go env)?
    linux amd64
  3. What did you do?
    http://play.golang.org/p/Fk7G5f3z1q
    The second template takes nil as its data, and accesses its .X field.
  4. What did you expect to see?
    I would expect the second template to return an error, just like the first:
    template: :1:2: executing "" at <.X>: can't evaluate field X in type int
  5. What did you see instead?
    The second template writes out "" and returns no error.
    This seems to correspond to the missingkey=default behaviour, which is documented to happen for maps. I don't think the empty interface should count as a map though, so I would have expected an error instead.

I can volunteer to provide a fix if the current behaviour is recognized to be a bug.

@walken-google walken-google changed the title text/template interprets interface{} as a map text/template interprets nil interface{} as a map Apr 18, 2016
@walken-google
Copy link
Contributor Author

@walken-google walken-google commented Apr 18, 2016

The second templates writes out "no value" with <> around it. Looks like I couldn't just say that in the initial comment.

@bradfitz bradfitz changed the title text/template interprets nil interface{} as a map text/template: interprets nil interface{} as a map Apr 18, 2016
@bradfitz bradfitz added this to the Unplanned milestone Apr 18, 2016
@adg
Copy link
Contributor

@adg adg commented Apr 19, 2016

Seems like a bug to me. Happy to review a fix.

@adg
Copy link
Contributor

@adg adg commented Apr 19, 2016

Oh sorry, there seems to be something missing in this report. You mention two programs, but I only see one?

Is <no value> not an acceptable output here?

@walken-google
Copy link
Contributor Author

@walken-google walken-google commented Apr 19, 2016

The program has two template calls in it - one with 0 as an argument, and one with nil. I would expect both to return similar errors, but the second succeeds. "no value" is documented behavior for maps, but not for the nil interface.

@adg
Copy link
Contributor

@adg adg commented Apr 19, 2016

Maybe I'm missing something, but the linked program only has one Execute call in it.

@walken-google
Copy link
Contributor Author

@walken-google walken-google commented Apr 19, 2016

Oh, sorry about that. Updated program is at http://play.golang.org/p/OaztWussVS - I did not realize that I needed to generate a new link after updating.

@adg
Copy link
Contributor

@adg adg commented Apr 19, 2016

This does seem like an inconsistency. I would expect both cases to return an error. But I think it has always been this way, and if we change it now, we might break existing Go programs, which we have promised we won't do.

@walken-google
Copy link
Contributor Author

@walken-google walken-google commented Apr 19, 2016

OK. we should probably close the issue then. Thanks!

@adg
Copy link
Contributor

@adg adg commented Apr 19, 2016

Thanks for the report.

@adg adg closed this Apr 19, 2016
@oec
Copy link

@oec oec commented Oct 19, 2016

I just stumbled over this myself and I find the behavior annoying. I understand that we can not change the current behavior without breaking a promise, but could we maybe add another option for template.Option like "missingkey=error, just error!" that would trigger to handle the nil-interface case differently? I'd be happy to work on a patch for this.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 21, 2016

CL https://golang.org/cl/31638 mentions this issue.

gopherbot pushed a commit that referenced this issue Nov 18, 2016
The existing implementation of text/template handles the option
"missingkey=error" in an inconsitent manner:  If the provided data is
a nil-interface, no error is returned (despite the fact that no key
can be found in it).

This patch makes text/template return an error if "missingkey=error"
is set and the provided data is a not a valid reflect.Value.

Fixes #15356

Change-Id: Ia0a83da48652ecfaf31f18bdbd78cb21dbca1164
Reviewed-on: https://go-review.googlesource.com/31638
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Oct 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.