text/template: index should return nil instead of index out of range error

[bep]

Please answer these questions before submitting your issue. Thanks!

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

go version go1.6 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/bep/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GO15VENDOREXPERIMENT="1"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fno-common"
CXX="clang++"
CGO_ENABLED="1"

1. What did you do?

package main

import (
    "bytes"
    "fmt"
    "html/template"
)

func main() {
    var data struct {
        A map[string]interface{}
        B []interface{}
    }

    t, err := template.New("a").Parse(`{{ with index .A "a" }}{{ else }}Not Found{{ end }}`)

    if err != nil {
        fmt.Println(err)
        return
    }
    var b bytes.Buffer
    err = t.Execute(&b, data)

    if err != nil {
        fmt.Println(err)
        return
    }

    fmt.Println("A:", b.String())
    b.Reset()

    t, err = template.New("b").Parse(`{{ with index .B 0}}{{ else }}Not Found{{ end }}`)

    if err != nil {
        fmt.Println(err)
        return
    }

    err = t.Execute(&b, data)

    if err != nil {
        fmt.Println(err)
        return
    }

    fmt.Println("B:", b.String())

}

1. What did you expect to see?

A: Not Found
B: Not Found

1. What did you see instead?

A: Not Found
template: b:1:8: executing "b" at <index .B 0>: error calling index: index out of range: 0

I understand why I get this error and I expect arguments ala "you can fix this by doing proper length checking of the slice. Or you can provide your own implementation of the index template func".

Sure. But it isn't always obvious in a template if I interact with a map or a slice, and with DRY in mind, I would say that range checking should be done in one place: Inside the index template func.

And the answer to the question "is this really a problem?":

I'm one of the maintainers in Hugo, probably the project with the largest amount of end-user-provided Go templates, and the number one most frequent support question by a large margin, is: "How do I access that parameter/argument/data in a safe way?"

gohugoio/hugo#1949

[louwers]

This is a huge pain indeed!
Is this supposed to work to check the length?

<li><a class="{{if (and (ge (len $.Path) 1) (eq (index $.Path 1) .))}}current{{end}}" href="courses/{{.}}/">{{.}}</a></li>

Edit: one off error, just further proves the point.
If you or anyone wrote a custom index function can be used in the meantime, it would be great if you could share it.

[bep]

https://github.com/spf13/hugo/blob/master/tpl/template_funcs.go#L1460

[rsc]

The current behavior matches Go itself: a map lookup for a missing key returns a zero value; a slice lookup for an invalid index panics. I don't see a strong argument for diverging from the language here. Does the template package replace panics with nils in other cases?

Also, the test is not that hard. Instead of:

{{with index .B 0}}{{else}}Not Found{{end}}

you write:

{{if lt 0 (len .B)}}{{index .B 0}}{{else}}Not Found{{end}}

Eliminating the panic will no doubt break other templates. This seems OK as is.

/cc @robpike in case he disagrees.

[bep]

@rsc I don't buy your argument that "the current behavior matches Go itself".

The signature for index is:

index(item interface{}, indices ...interface{}) (interface{}, error)

It is a helper func full of reflection to make it usable in many situations (slices of anything, maps of anything). And when you live in a template world where everything is an interface{} I think it is up to the helper funcs to help avoiding a mess of nested conditionals.

You cannot write:

{{if lt 0 (len .B)}}{{index .B .C}}{{else}}Not Found{{end}}

Without knowing that .B is a slice and .C is an int.

index knows this and should be able to take advantage of that to help the end user.

[rsc]

What I mean is that {{index x y}} is behaving exactly like Go's x[y], and similarly {{index x y z}} is behaving like x[y][z] and so on.

You are arguing that index should diverge from Go's definition of those operations. As I said above:

I don't see a strong argument for diverging from the language here. Does the template package replace panics with nils in other cases?

[bep]

Does the template package replace panics with nils in other cases?

There are plenty of cases where the template package avoids panics, which is a more relevant question:

if idx < 0 || idx + 1 > len(s)  {
   return nil // avoid panic!
}

But it is up to you. The workaround for people in the wild is to maintain forks of the index func.

[rsc]

@bep, please point to specific instances. I can't find the code snippet you posted in the text/template package nor the html/template package. In fact I can't find the string 'idx' in either of those at all.

[bep]

@rsc it was just a general remark -- as I assume that html/template and text/template were full of slice bounds checking (I would even consider the for i := 0; i < len(mySlice) loop to fall into that category) to prevent panics, but if that is not the case I'm impressed.