Escaped quotes are not preserved
rocurley opened this issue · 8 comments
Expected behaviour
Tags with escaped quotes still contain escaped quotes after parsing and re-serialization.
Observed behaviour
Escapes are stripped from quotes.
Failing Test
package main
import (
"reflect"
"testing"
"github.com/fatih/structtag"
"github.com/stretchr/testify/require"
)
type ty struct {
t string `tag:"foo,bar:\"baz\""`
}
func TestParse(t *testing.T) {
tagString := string(reflect.TypeOf(ty{}).Field(0).Tag)
tags, err := structtag.Parse(tagString)
require.NoError(t, err)
tag, err := tags.Get("tag")
require.NoError(t, err)
require.Equal(t, `bar:"baz"`, tag.Options[0])
require.Equal(t, tagString, tags.String())
}Test Results
--- FAIL: TestParse (0.00s)
main_test.go:22:
Error Trace: main_test.go:22
Error: Not equal:
expected: "tag:\"foo,bar:\\\"baz\\\"\""
actual : "tag:\"foo,bar:\"baz\"\""
Hi @rocurley
Thanks for the issue. I've added the following test case:
{
name: "tag with quoted name",
tag: `json:"foo,bar:\"baz\""`,
exp: []*Tag{
{
Key: "json",
Name: "foo",
Options: []string{`bar:\"baz\"`},
},
},
},
and this is what I get:
--- FAIL: TestParse (0.00s)
--- FAIL: TestParse/tag_with_quoted_name (0.00s)
tags_test.go:183: parse
want: []*structtag.Tag{{
Key: 'json',
Name: 'foo',
Option: 'bar:\"baz\"',
}}
got : []*structtag.Tag{{
Key: 'json',
Name: 'foo',
Option: 'bar:"baz"',
}}
What do you look for? The escaped quotes are stripped from the result.
Sorry, my initial report had a mistake, which I've corrected. I agree that the parsing behaviour is good: the options are unescaped, which seems like the sanest default. The issue is that the escaping isn't added back in when writing out struct tags. This means that parsing and then printing tags should yield the original tags (which is what the final test case in my example test checks).
Hey @fatih , I think that solves the problem where parsing and printing changes the struct. However, I think you were right earlier that the strings should be presented to users with unescaped. I think you could get both by re-quoting strings when printing them out, as done here. From the thread, it looks like you'd prefer not to merge the whole PR, but it might make sense to merge just that line.
So what I see as the problem is that, when you parse a tag, then print it back, the tag is changed, even if you don't manipulate it. Your fix does fix that issue, but also changes how options are parsed. I'd instead prefer to change how tags are printed. I agree that the other PR has a lot of unrelated changes, but it also fixes this problem by changing how a tag is printed:
From:
return fmt.Sprintf(`%s:"%s"`, t.Key, t.Value())
To:
return fmt.Sprintf(`%s:%q`, t.Key, t.Value())
Here's a summary of the current state, your fix, and my preferred outcome:
Currently
Tag:
`json:"foo,bar:\"baz\""`
Becomes:
[]*Tag{
{
Key: "json",
Name: "foo",
Options: []string{`bar:"baz"`},
},
}
And if you print the tag, it yields:
`json:"foo,bar:"baz""`
Your proposed fix
Tag:
`json:"foo,bar:\"baz\""`
Becomes:
[]*Tag{
{
Key: "json",
Name: "foo",
Options: []string{`bar:\"baz\"`},
},
}
And if you print the tag, it yields:
`json:"foo,bar:\"baz\""`
My preferred outcome
Tag:
`json:"foo,bar:\"baz\""`
Becomes:
[]*Tag{
{
Key: "json",
Name: "foo",
Options: []string{`bar:"baz"`},
},
}
And if you print the tag, it yields:
`json:"foo,bar:\"baz\""`