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, html/template: nil argument to function yields wrong number of args error #18716

Closed
sethvargo opened this issue Jan 19, 2017 · 13 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@sethvargo
Copy link
Contributor

sethvargo commented Jan 19, 2017

What version of Go are you using (go version)?

go version go1.7.4 darwin/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/sethvargo/Development/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.7.4_2/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.7.4_2/libexec/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/d0/g1brm3hj0431461p8cxqtjth0000gn/T/go-build267389745=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

https://play.golang.org/p/R3CMVl_nmx

What did you expect to see?

Passing a nested field in a struct that is nil should pass in the zero value for the accepting function. I either expected a different error, or I expected text/template to coerce the value into the signature of the receiving function (string in this case), and use the string zero value.

What did you see instead?

An error is returned indicating an incorrect number of arguments was provided. This is especially confusing because, even if the zero value is provided, the result of the function call should be nil. At worst you're passing nil to a function that accepts a string, but that's not "incorrect number of args", it's a TypeError.

Related Code

From my tracing, the code appears to be here.

@sethvargo
Copy link
Contributor Author

Even if I initialize the map and/or the map key, the error message is still the same if the provided value is nil: https://play.golang.org/p/lpFcNJYNdu

@sethvargo
Copy link
Contributor Author

Further playing has narrowed it down to

if final.IsValid() {
numIn++
}
. In most cases, it looks like final is treated as an argument (input) to the function. In the case that the value is nil, it's not "valid" and therefore does not increment the number of valid args.

@josharian josharian changed the title text/template map yields wrong number of args for empty values text/template: map yields wrong number of args for empty values Jan 20, 2017
@paranoiacblack paranoiacblack changed the title text/template: map yields wrong number of args for empty values text/template: nil argument to function yields wrong number of args error Feb 6, 2017
@paranoiacblack
Copy link
Contributor

I updated the description since this is not specific to maps, per se. Passing nil in any way to template-evaluated function causes it to exit early rather than type-checking the argument and giving the error you expect. Consider the following example: https://play.golang.org/p/PkAsQyklDW.

It seems like it might be a good idea to try to allow nil specifically to be used as an argument although reflect.ValueOf(nil) is !reflect.Value.IsValid.

@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 21, 2017
@bradfitz bradfitz added this to the Go1.9 milestone Mar 21, 2017
@bradfitz
Copy link
Contributor

/cc @robpike

@robpike
Copy link
Contributor

robpike commented Mar 21, 2017

This is unchanged since 1.4 and probably the beginning, so no urgency but it is annoying. Not a regression, in other words.

Looks like a bug to me, might be hard to fix because nil is poorly handled by reflect and the template code doesn't do much to improve the situation.

@odeke-em
Copy link
Member

Same problem for html/template, by modifying @paranoiacblack's test program in #18716 (comment) to import "html/template" instead i.e https://play.golang.org/p/Kf4gtQWnOn

@odeke-em odeke-em changed the title text/template: nil argument to function yields wrong number of args error text/template, html/template: nil argument to function yields wrong number of args error May 20, 2017
@rsc
Copy link
Contributor

rsc commented May 22, 2017

Clearly this is the wrong error, but it should be some error, so this can wait until Go 1.10.

@rsc rsc modified the milestones: Go1.10, Go1.9 May 22, 2017
@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 22, 2017
@odeke-em
Copy link
Member

Ooh unfortunately we didn't get a CL in for Go1.10, @sethvargo might you be interested in solving this for Go1.11? Or @rsc is this still open game for Go1.10?

@sethvargo
Copy link
Contributor Author

Hey @odeke-em

I tried to solve it myself, but unfortunately I got lost in the reflection bits 😦

@odeke-em
Copy link
Member

Not a problem @sethvargo, I had asked because, since you had been investigating it, it'd be nice to carry on the ownership/credit and dibs :) We can take a look at it for Go1.11. Thank you!

@odeke-em
Copy link
Member

@rsc am punting it to Go1.11, please feel free to rollback.

@odeke-em odeke-em modified the milestones: Go1.10, Go1.11 Nov 23, 2017
@mvdan mvdan self-assigned this Feb 13, 2018
@gopherbot
Copy link

Change https://golang.org/cl/95215 mentions this issue: text/template: differentiate nil from missing arg

@gopherbot
Copy link

Change https://golang.org/cl/121815 mentions this issue: html/template: ignore untyped nil arguments to default escapers

gopherbot pushed a commit that referenced this issue Jul 9, 2018
CL 95215 changed text/template so that untyped nil arguments were no
longer ignored, but were instead passed to functions as expected.
This had an unexpected effect on html/template, where all data is
implicitly passed to functions: originally untyped nil arguments were
not passed and were thus effectively ignored, but after CL 95215 they
were passed and were printed, typically as an escaped version of "<nil>".

This CL restores some of the behavior of html/template by ignoring
untyped nil arguments passed implicitly to escaper functions.

While eliminating one change to html/template relative to earlier
releases, this unfortunately introduces a different one: originally
values of interface type with the value nil were printed as an escaped
version of "<nil>". With this CL they are ignored as though they were
untyped nil values. My judgement is that this is a less common case.
We'll see.

This CL adds some tests of typed and untyped nil values to
html/template and text/template to capture the current behavior.

Updates #18716
Fixes #25875

Change-Id: I5912983ca32b31ece29e929e72d503b54d7b0cac
Reviewed-on: https://go-review.googlesource.com/121815
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jun 30, 2019
@rsc rsc unassigned mvdan Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

8 participants