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

Escape sequences are being parsed inside strings #73

Closed
RangelReale opened this issue Mar 16, 2018 · 14 comments
Closed

Escape sequences are being parsed inside strings #73

RangelReale opened this issue Mar 16, 2018 · 14 comments

Comments

@RangelReale
Copy link

Another problem I found, given this definition:

string name  = 3 [(validate.rules).string = {
                  pattern:   "^[^\d\s]+( [^\d\s]+)*$",
                  max_bytes: 256,
               }];

The parser is trying to escape the "\d" sequence in "pattern", but I don't think it should interpret the string, shouldn't it leave this to the library user?
If I added another "" after the other ones, then it works, but the output string has the two "", it even doesn't remove them.

2018/03/16 09:08:55 go scanner error at <input>:11:34 = illegal char escape
go scanner error at <input>:11:34 = illegal char escape
go scanner error at <input>:11:34 = illegal char escape
go scanner error at <input>:11:34 = illegal char escape
@emicklei
Copy link
Owner

this is a challenging problem. Need to think about how to solve it. First idea, do the scanning of option content differently.

@RangelReale
Copy link
Author

Debugging into the library, I had the impression that this is done by Go, by the Scanner interface if I'm not wrong.

@emicklei
Copy link
Owner

true, so apparently the option syntax does not conform to the Go language spec (which it should not ofcourse but saved me a lot of work, initially).

@emicklei
Copy link
Owner

i created a test for this but was surprised to see a PASS

func TestFieldOptionEscapeCharacters(t *testing.T) {
	src := `string name  = 3 [(validate.rules).string = {
	pattern:   "^[^\d\s]+( [^\d\s]+)*$",
	max_bytes: 256,
 }];`
	p := newParserOn(src)
	f := newNormalField()
	err := f.parse(p)
	if err != nil {
		t.Fatal(err)
	}
}

@RangelReale
Copy link
Author

This is the file that gives errors to me (using the lyft validator):

syntax = "proto3";

import "validate/validate.proto";

message Person {
  uint64 id    = 1 [(validate.rules).uint64.gt    = 999];

  string email = 2 [(validate.rules).string.email = true];

  string name  = 3 [(validate.rules).string = {
                      pattern:   "^[^\d\s]+( [^\d\s]+)*$",
                      max_bytes: 256,
                   }];

  Location home = 4 [(validate.rules).xmessage.required = true];

  message Location {
    double lat = 1 [(validate.rules).double = { gte: -90,  lte: 90 }];
    double lng = 2 [(validate.rules).double = { gte: -180, lte: 180 }];
  }
}

@emicklei
Copy link
Owner

what go sdk version are you using?

@emicklei
Copy link
Owner

emicklei commented Mar 16, 2018

so when testing the actual value in Go, I needed to escape the slashes

func TestFieldOptionEscapeCharacters(t *testing.T) {
	src := `string name  = 3 [(validate.rules).string = {
	pattern:   "^[^\d\s]+( [^\d\s]+)*$",
	max_bytes: 256,
 }];`
	p := newParserOn(src)
	f := newNormalField()
	err := f.parse(p)
	if err != nil {
		t.Fatal(err)
	}
	if got, want := f.Options[0].AggregatedConstants[0].Source, "^[^\\d\\s]+( [^\\d\\s]+)*$"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
}

I think my parser is accepting it because of scanner.ScanRawStrings when initializing the scanner.

so i don't know how you found this problem (and i do not), yet.

@RangelReale
Copy link
Author

go version
go version go1.9.2 windows/amd64

I will try on the latest version.

@RangelReale
Copy link
Author

I had the same problem on the last version.

I am uploading my test source and file, running "go run main.go" on this directory triggers the errors I talked above.

tstproto.zip

@emicklei
Copy link
Owner

thanks, this indeed reproduces the problem. I will find the differences later

@emicklei
Copy link
Owner

thx, i know now why I did not see the errors before. Now I am working on a solution.

@emicklei
Copy link
Owner

see #74

@RangelReale
Copy link
Author

Nice, thanks!

emicklei added a commit that referenced this issue Mar 18, 2018
* ignore illegal escape chars while parsing option constant
@emicklei
Copy link
Owner

can this be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants