code review 5683077: go/parser: check import path restrictions

go/parser: check import path restrictions

Replaces pending CL 5674097.
Thanks to ality@pbrane.org for spearheading
the effort.

[gri, 2012-02-22 20:59:40]

Hello rsc@golang.org, r@golang.org (cc: golang-dev@googlegroups.com),

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

[rsc, 2012-02-22 21:03:06]

LGTM but wait for r

http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser.go
File src/pkg/go/parser/parser.go (right):

http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser.go#n...
src/pkg/go/parser/parser.go:1923: if strings.ContainsRune(illegalChars, r) ||
unicode.IsControl(r) || unicode.IsSpace(r) {
|| r == unicode.ReplacementChar

http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser_test.go
File src/pkg/go/parser/parser_test.go (right):

http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser_test...
src/pkg/go/parser/parser_test.go:221: "`a`":      false,
add
"\x80\x80": false,

[r, 2012-02-22 21:08:07]

http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser.go
File src/pkg/go/parser/parser.go (right):

http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser.go#n...
src/pkg/go/parser/parser.go:1923: if strings.ContainsRune(illegalChars, r) ||
unicode.IsControl(r) || unicode.IsSpace(r) {
On 2012/02/22 21:03:07, rsc wrote:
> || r == unicode.ReplacementChar

just put the replacement char in the illegalChars string: \uFFFD
the others are fine as written

http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser.go#n...
src/pkg/go/parser/parser.go:1947: p.error(p.pos, "invalid import path")
can you include the erroneous path in the error?

[gri, 2012-02-22 21:13:34]

PTAL
- gri

On Wed, Feb 22, 2012 at 1:08 PM,  <r@golang.org> wrote:
>
> http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser.go
> File src/pkg/go/parser/parser.go (right):
>
>
http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser.go#n...
> src/pkg/go/parser/parser.go:1923: if strings.ContainsRune(illegalChars,
> r) || unicode.IsControl(r) || unicode.IsSpace(r) {
> On 2012/02/22 21:03:07, rsc wrote:
>>
>> || r == unicode.ReplacementChar
>
>
> just put the replacement char in the illegalChars string: \uFFFD
> the others are fine as written
>
>
http://codereview.appspot.com/5683077/diff/1002/src/pkg/go/parser/parser.go#n...
> src/pkg/go/parser/parser.go:1947: p.error(p.pos, "invalid import path")
> can you include the erroneous path in the error?
>
> http://codereview.appspot.com/5683077/

[r, 2012-02-22 21:22:07]

LGTM

[gri, 2012-02-23 07:21:59]

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

go/parser: check import path restrictions

Replaces pending CL 5674097.
Thanks to ality@pbrane.org for spearheading
the effort.

R=rsc, r
CC=golang-dev
http://codereview.appspot.com/5683077