code review 7311061: syscall: check for invalid characters in Setenv on Unix

syscall: check for invalid characters in Setenv on Unix

On POSIX, '=' in key is explicitly invalid, and '\x00' in key/value is implicitly invalid.

[speter, 2013-02-08 13:43:40]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[iant, 2013-02-08 14:50:32]

I'm not sure whether this is necessary or not.

https://codereview.appspot.com/7311061/diff/7001/src/pkg/syscall/env_unix.go
File src/pkg/syscall/env_unix.go (right):

https://codereview.appspot.com/7311061/diff/7001/src/pkg/syscall/env_unix.go#...
src/pkg/syscall/env_unix.go:74: for _, b := range []byte(key) {
This will convert the string to []byte, using memory.  Write
    for i := 0; i < len(key); i++
and test key[i].

[bradfitz, 2013-02-08 15:28:49]

On Fri, Feb 8, 2013 at 6:50 AM, <iant@golang.org> wrote:

> I'm not sure whether this is necessary or not.
>

I probably wouldn't change the Setenv signature for this or introduce a new
error type, but given that this already returns EINVAL, this seems fine to
me.  (except for the allocation you mentioned).

[speter, 2013-02-08 17:20:39]

Please take another look.

> I'm not sure whether this is necessary or not.

Not checking for '\x00' is inconsistent with the rest of the syscall functions
(which return EINVAL), and not checking for '=' is inconsistent with the OS --
at least with linux, freebsd, openbsd, and netbsd. For darwin, a man page I
found for setenv(3) indicates that it tolerates a trailing '=' appended to the
key, but IIUC the equivalent has never been supported or considered for support
in Go.

https://codereview.appspot.com/7311061/diff/7001/src/pkg/syscall/env_unix.go
File src/pkg/syscall/env_unix.go (right):

https://codereview.appspot.com/7311061/diff/7001/src/pkg/syscall/env_unix.go#...
src/pkg/syscall/env_unix.go:74: for _, b := range []byte(key) {
On 2013/02/08 14:50:32, iant wrote:
> This will convert the string to []byte, using memory.  Write
>     for i := 0; i < len(key); i++
> and test key[i].

Done.

[iant, 2013-02-08 18:45:40]

LGTM

[iant, 2013-02-08 18:45:51]

*** Submitted as https://code.google.com/p/go/source/detail?r=9d68dabd0026 ***

syscall: check for invalid characters in Setenv on Unix

On POSIX, '=' in key is explicitly invalid, and '\x00' in key/value is
implicitly invalid.

R=golang-dev, iant, bradfitz
CC=golang-dev
https://codereview.appspot.com/7311061

Committer: Ian Lance Taylor <iant@golang.org>