Skip to content

Commit

Permalink
THRIFT-5767: use string builder to parse strings with escaped quotes
Browse files Browse the repository at this point in the history
Client: Go
  • Loading branch information
k-walton committed Mar 18, 2024
1 parent 58c2785 commit 9665045
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 25 deletions.
5 changes: 4 additions & 1 deletion lib/go/test/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ gopath: $(THRIFT) $(THRIFTTEST) \
ProcessorMiddlewareTest.thrift \
ClientMiddlewareExceptionTest.thrift \
ValidateTest.thrift \
ForwardType.thrift
ForwardType.thrift \
StringParseAllocationTest.thrift
mkdir -p gopath/src
grep -v list.*map.*list.*map $(THRIFTTEST) | grep -v 'set<Insanity>' > ThriftTest.thrift
$(THRIFT) $(THRIFTARGS) -r IncludesTest.thrift
Expand Down Expand Up @@ -98,6 +99,7 @@ gopath: $(THRIFT) $(THRIFTTEST) \
$(THRIFT) $(THRIFTARGS) ClientMiddlewareExceptionTest.thrift
$(THRIFT) $(THRIFTARGS) ValidateTest.thrift
$(THRIFT) $(THRIFTARGS) ForwardType.thrift
$(THRIFT) $(THRIFTARGS) StringParseAllocationTest.thrift
ln -nfs ../../tests gopath/src/tests
cp -r ./dontexportrwtest gopath/src
touch gopath
Expand Down Expand Up @@ -169,6 +171,7 @@ EXTRA_DIST = \
RefAnnotationFieldsTest.thrift \
RequiredFieldTest.thrift \
ServicesTest.thrift \
StringParseAllocationTest.thrift \
TypedefFieldTest.thrift \
UnionBinaryTest.thrift \
UnionDefaultValueTest.thrift \
Expand Down
22 changes: 22 additions & 0 deletions lib/go/test/StringParseAllocationTest.thrift
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

struct StringStruct {
1: required string example
}
43 changes: 43 additions & 0 deletions lib/go/test/tests/string_parse_allocation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package tests

import (
"context"
"fmt"
"strings"
"testing"

"github.com/apache/thrift/lib/go/test/gopath/src/stringparseallocationtest"
"github.com/apache/thrift/lib/go/thrift"
)

func TestSimpleJsonStringParse_Allocations(t *testing.T) {
byteAllocationLimit := 100 * 1024 // 100 KB
res := testing.Benchmark(BenchmarkSimpleJsonStringParse_Allocations)
if res.AllocedBytesPerOp() > int64(byteAllocationLimit) {
t.Errorf("Total memory allocation size too high: %d (> %d)", res.AllocedBytesPerOp(), byteAllocationLimit)
}
}

func BenchmarkSimpleJsonStringParse_Allocations(b *testing.B) {
b.ReportAllocs()
b.StopTimer()
numEscapedQuotes := 1000
var sb strings.Builder
for i := 0; i < numEscapedQuotes; i++ {
sb.WriteString(`\"`)
}

testString := fmt.Sprintf(`{"1": {"str": "this is a test with %d of escaped quotes %s"}}`, numEscapedQuotes, sb.String())
stringStruct := stringparseallocationtest.NewStringStruct()
transport := thrift.NewTMemoryBuffer()
p := thrift.NewTJSONProtocol(transport)

for i := 0; i < b.N; i++ {
transport.Reset()
transport.WriteString(testString)
transport.Flush(context.Background())
b.StartTimer()
_ = stringStruct.Read(context.Background(), p)
b.StopTimer()
}
}
43 changes: 19 additions & 24 deletions lib/go/thrift/simple_json_protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"io"
"math"
"strconv"
"strings"
)

type _ParseContext int
Expand Down Expand Up @@ -922,15 +923,7 @@ func (p *TSimpleJSONProtocol) ParseStringBody() (string, error) {
if err != nil {
return "", NewTProtocolException(err)
}
l := len(line)
// count number of escapes to see if we need to keep going
i := 1
for ; i < l; i++ {
if line[l-i-1] != '\\' {
break
}
}
if i&0x01 == 1 {
if endsWithoutEscapedQuote(line) {
v, ok := jsonUnquote(string(JSON_QUOTE) + line)
if !ok {
return "", NewTProtocolException(err)
Expand All @@ -951,27 +944,29 @@ func (p *TSimpleJSONProtocol) ParseStringBody() (string, error) {
}

func (p *TSimpleJSONProtocol) ParseQuotedStringBody() (string, error) {
line, err := p.reader.ReadString(JSON_QUOTE)
if err != nil {
return "", NewTProtocolException(err)
var sb strings.Builder

for {
line, err := p.reader.ReadString(JSON_QUOTE)
if err != nil {
return "", NewTProtocolException(err)
}
sb.WriteString(line)
if endsWithoutEscapedQuote(line) {
return sb.String(), nil
}
}
l := len(line)
// count number of escapes to see if we need to keep going
}

func endsWithoutEscapedQuote(s string) bool {
l := len(s)
i := 1
for ; i < l; i++ {
if line[l-i-1] != '\\' {
if s[l-i-1] != '\\' {
break
}
}
if i&0x01 == 1 {
return line, nil
}
s, err := p.ParseQuotedStringBody()
if err != nil {
return "", NewTProtocolException(err)
}
v := line + s
return v, nil
return i&0x01 == 1
}

func (p *TSimpleJSONProtocol) ParseBase64EncodedBody() ([]byte, error) {
Expand Down

0 comments on commit 9665045

Please sign in to comment.