code review 5010044: http/cgi: clean up environment.

http/cgi: clean up environment.
clean up duplicate environment for CGI.
overriding former by latter.
On windows, When there are duplicated environments like following,

SCRIPT_FILENAME=c:/progra~1/php/php-cgi.exe
SCRIPT_FILENAME=/foo.php

CreateProcess use first entry.

If make cgi.Handle like following,

        cgih = cgi.Handler{
                Path: "c:/strawberry/perl/bin/perl.exe",
                Dir:  "c:/path/to/webroot",
                Root: "c:/path/to/webroot",
                Args: []string{"foo.php"},
                Env:  []string{"SCRIPT_FILENAME=foo.php"},
        }

http/cgi should behave "SCRIPT_FILENAME is foo.php".
But currently, http/cgi is set duplicate environment entries.
So, browser show binary dump of "php-cgi.exe" that is specified indented
SCRIPT_FILENAME in first entry.
This change clean up duplicates, and use latters.

[mattn, 2011-09-15 09:20:45]

Hello golang-dev@googlecode.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

I'd like you to review this change to
http://go.googlecode.com/hg/

[bradfitz, 2011-09-15 17:53:14]

Add a little test too?

http://codereview.appspot.com/5010044/diff/4001/src/pkg/http/cgi/host.go
File src/pkg/http/cgi/host.go (right):

http://codereview.appspot.com/5010044/diff/4001/src/pkg/http/cgi/host.go#newc...
src/pkg/http/cgi/host.go:71: func cleanEnv(env []string) (ret []string) {
"cleanEnv" is a pretty ambiguous name. (it sounds almost like "do stuff")  I'd
add a little comment above this, describing its goal.

http://codereview.appspot.com/5010044/diff/4001/src/pkg/http/cgi/host.go#newc...
src/pkg/http/cgi/host.go:72: l := len(env)
I don't like "l" as a variable name.  At least here in codereview it looks like
a "1".  Use "n"?

http://codereview.appspot.com/5010044/diff/4001/src/pkg/http/cgi/host.go#newc...
src/pkg/http/cgi/host.go:171: env = cleanEnv(env)
env = removeLeadingDuplicates(env)

might read more obviously here

[mattn, 2011-09-16 02:10:03]

Hello golang-dev@googlecode.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[mattn, 2011-09-16 02:12:16]

http://codereview.appspot.com/5010044/diff/4001/src/pkg/http/cgi/host.go
File src/pkg/http/cgi/host.go (right):

http://codereview.appspot.com/5010044/diff/4001/src/pkg/http/cgi/host.go#newc...
src/pkg/http/cgi/host.go:71: func cleanEnv(env []string) (ret []string) {
On 2011/09/15 17:53:14, bradfitz wrote:
> "cleanEnv" is a pretty ambiguous name. (it sounds almost like "do stuff")  I'd
> add a little comment above this, describing its goal.

Done.

http://codereview.appspot.com/5010044/diff/4001/src/pkg/http/cgi/host.go#newc...
src/pkg/http/cgi/host.go:72: l := len(env)
On 2011/09/15 17:53:14, bradfitz wrote:
> I don't like "l" as a variable name.  At least here in codereview it looks
like
> a "1".  Use "n"?

Done.

http://codereview.appspot.com/5010044/diff/4001/src/pkg/http/cgi/host.go#newc...
src/pkg/http/cgi/host.go:171: env = cleanEnv(env)
On 2011/09/15 17:53:14, bradfitz wrote:
> env = removeLeadingDuplicates(env)
> 
> might read more obviously here

Done.

[bradfitzgoog, 2011-09-16 17:36:48]

LGTM

[bradfitz, 2011-09-16 17:36:58]

*** Submitted as http://code.google.com/p/go/source/detail?r=68ba2ebd1645 ***

http/cgi: clean up environment.
clean up duplicate environment for CGI.
overriding former by latter.
On windows, When there are duplicated environments like following,

SCRIPT_FILENAME=c:/progra~1/php/php-cgi.exe
SCRIPT_FILENAME=/foo.php

CreateProcess use first entry.

If make cgi.Handle like following,

        cgih = cgi.Handler{
                Path: "c:/strawberry/perl/bin/perl.exe",
                Dir:  "c:/path/to/webroot",
                Root: "c:/path/to/webroot",
                Args: []string{"foo.php"},
                Env:  []string{"SCRIPT_FILENAME=foo.php"},
        }

http/cgi should behave "SCRIPT_FILENAME is foo.php".
But currently, http/cgi is set duplicate environment entries.
So, browser show binary dump of "php-cgi.exe" that is specified indented
SCRIPT_FILENAME in first entry.
This change clean up duplicates, and use latters.

R=golang-dev, bradfitz, bradfitz
CC=golang-dev
http://codereview.appspot.com/5010044

Committer: Brad Fitzpatrick <bradfitz@golang.org>