Commit b28613c2 authored by Kirill Smelkov's avatar Kirill Smelkov Committed by Kamil Kisiel

Add StrictUnicode mode

Up till now ogórek works relatively well for me but experience gained
with ZODB/go and Wendelin.core highlighted two problems with strings:

1) loading, then re-saving string data is not generally identity, and
2) there is no way to distinguish binary data saved via py2 str from
   unicode strings saved via either py2 unicode or py3 str.

Let me explain those problems:

1) Loading, then re-saving string data is not generally identity
----------------------------------------------------------------

On decoding ogórek currently loads both byte-string (*STRING opcodes)
and unicode string (*UNICODE opcodes) into the same Go type string. And
on encoding that string type is encoded as *STRING for protocol <= 2 and
as *UNICODE for protocol >= 3. It was me to implement that encoding
logic in 2018 in e7d96969 (encoder: Fix string wrt protocol version)
where in particular I did

    - if protocol >= 3 we have to emit the string as unicode pickle object
      the same way as Python3 does. If we don't do - Python3 won't be
      generally able to load our pickle ...

with the idea that protocol=3 always means that the pickle is intended
to be for Python3.

But there I missed that zodbpickle can use and generate pickles with
protocol=3 even on py2 and that ZODB/py2 actually uses this protocol=3
mode since https://github.com/zopefoundation/ZODB/commit/12ee41c4
authored in the same 2018.

So, there can be pickles saved under protocol=3 that do contain both
*STRING and *UNICODE opcodes, and when ogórek sees those it loads that
*STRING and *UNICODE data as just string loosing the information about
type of particular variant. And then on encoding the data is saved as
all UNICODE, or all STRING, thus breaking decode/encode=identity
property.

This breakage is there even with plain pickle and protocol=2 - when both
*STRING and *UNICODE are in the database, ogórek loads them both as the
same Go type string, and then saving back under the same protocol=2 goes
as all STRING resulting in unicode objects becoming str on resave
without any intended change.

2) there is no way to distinguish binary data saved via py2 str from unicode strings saved via either py2 unicode or py3 str
----------------------------------------------------------------------------------------------------------------------------

Continuing above example of py2 database with both *STRING and *UNICODE
opcodes present there is currently no way for application to distinguish
those from each other. In other words there is currently no way for the
application to distinguish whether it is binary data coming from py2
protocol <= 2 era, from unicode text.

The latter problem I hit for real: with Wendelin.core we have lots of
data saved from under Python2 as just str. And the Go part of
Wendlin.core, upon loading block of data, wants to accept only binary -
either bytes from py3 or bytestring from py2, but not unicode, because
it indicates a mistake if e.g. a ZBlk object would come with unicode data

https://lab.nexedi.com/nexedi/wendelin.core/-/blob/07087ec8/bigfile/file_zodb.py#L267-300
https://lab.nexedi.com/nexedi/wendelin.core/-/blob/07087ec8/wcfs/internal/zdata/zblk.go#L31-32
https://lab.nexedi.com/nexedi/wendelin.core/-/blob/07087ec8/wcfs/internal/zdata/zblk.go#L98-107

but there is currently no way to distinguish whether it was unicode or
bytestring saved into the database becase they both are represented as
the same Go string type after decoding.

----------------------------------------

So to solve those problems I thought it over and understood that the
issues start to appear becase we let *STRING and *UNICODE to become
mixed into the same entity on loading. This behaviour is there from
ogórek beginning and the intention, it seems, was to get the data from
the pickle stream in an easily-accepted form on the Go side. However the
convenience turned out to come with cost of loosing some correctness in
the general case as explained above.

So if we are to fix the correctness we need to change that behaviour and
load *STRING and *UNICODE opcodes into distinct types, so that the
information about what was what is preserved and it becomes possible to
distinguish bytestring from unicode strings and resave the data in
exactly the same form as loaded. Though we can do this only under opt-in
option with default behaviour staying as it was before to preserve
backward compatibility.

-> Do it.

Below is excerpt from doc.go and DecoderConfig and EncoderConfig changes that
describe the new system:

    For strings there are two modes. In the first, default, mode both py2/py3
    str and py2 unicode are decoded into string with py2 str being considered
    as UTF-8 encoded. Correspondingly for protocol ≤ 2 Go string is encoded as
    UTF-8 encoded py2 str, and for protocol ≥ 3 as py3 str / py2 unicode.
    ogórek.ByteString can be used to produce bytestring objects after encoding
    even for protocol ≥ 3. This mode tries to match Go string with str type of
    target Python depending on protocol version, but looses information after
    decoding/encoding cycle:

        py2/py3 str    string                       StrictUnicode=n mode, default
        py2 unicode  →  string
        py2 str      ←  ogórek.ByteString

    However with StrictUnicode=y mode there is 1-1 mapping in between py2
    unicode / py3 str vs Go string, and between py2 str vs ogórek.ByteString.
    In this mode decoding/encoding and encoding/decoding operations are always
    identity with respect to strings:

        py2 unicode / py3 str    string             StrictUnicode=y mode
        py2 str                  ogórek.ByteString

    For bytes, unconditionally to string mode, there is direct 1-1 mapping in
    between Python and Go types:

        bytes          ogórek.Bytes   (~)
        bytearray      []byte

    --------

    type DecoderConfig struct {
        // StrictUnicode, when true, requests to decode to Go string only
        // Python unicode objects. Python2 bytestrings (py2 str type) are
        // decoded into ByteString in this mode...
        StrictUnicode bool
    }

    type EncoderConfig struct {
        // StrictUnicode, when true, requests to always encode Go string
        // objects as Python unicode independently of used pickle protocol...
        StrictUnicode bool
    }

Since strings are now split into two types, string and ByteString, and
ByteString can either mean text or binary data, new AsString and AsBytes
helpers are also added to handle string and binary data in uniform way
supporting both py2 and py3 databases. Corresponding excerpts from
doc.go and typeconv.go changes with the description of those helpers
come below:

    On Python3 strings are unicode strings and binary data is represented by
    bytes type. However on Python2 strings are bytestrings and could contain
    both text and binary data. In the default mode py2 strings, the same way as
    py2 unicode, are decoded into Go strings. However in StrictUnicode mode py2
    strings are decoded into ByteString - the type specially dedicated to
    represent them on Go side. There are two utilities to help programs handle
    all those bytes/string data in the pickle stream in uniform way:

        - the program should use AsString if it expects text   data -
          either unicode string, or byte string.
        - the program should use AsBytes  if it expects binary data -
          either bytes, or byte string.

    Using the helpers fits into Python3 strings/bytes model but also allows to
    handle the data generated from under Python2.

    --------

    // AsString tries to represent unpickled value as string.
    //
    // It succeeds only if the value is either string, or ByteString.
    // It does not succeed if the value is Bytes or any other type.
    //
    // ByteString is treated related to string because ByteString represents str
    // type from py2 which can contain both string and binary data.
    func AsString(x interface{}) (string, error) {

    // AsBytes tries to represent unpickled value as Bytes.
    //
    // It succeeds only if the value is either Bytes, or ByteString.
    // It does not succeed if the value is string or any other type.
    //
    // ByteString is treated related to Bytes because ByteString represents str
    // type from py2 which can contain both string and binary data.
    func AsBytes(x interface{}) (Bytes, error) {

ZODB/go and Wendelin.core intend to switch to using StrictUnicode mode
while leaving ogórek to remain 100% backward-compatible in its default
mode for other users.
parent 010fbd2e
......@@ -26,11 +26,37 @@
// tuple ↔ ogórek.Tuple
// dict ↔ map[interface{}]interface{}
//
// str ↔ string (+)
//
// For strings there are two modes. In the first, default, mode both py2/py3
// str and py2 unicode are decoded into string with py2 str being considered
// as UTF-8 encoded. Correspondingly for protocol ≤ 2 Go string is encoded as
// UTF-8 encoded py2 str, and for protocol ≥ 3 as py3 str / py2 unicode.
// ogórek.ByteString can be used to produce bytestring objects after encoding
// even for protocol ≥ 3. This mode tries to match Go string with str type of
// target Python depending on protocol version, but looses information after
// decoding/encoding cycle:
//
// py2/py3 str ↔ string StrictUnicode=n mode, default
// py2 unicode → string
// py2 str ← ogórek.ByteString
//
// However with StrictUnicode=y mode there is 1-1 mapping in between py2
// unicode / py3 str vs Go string, and between py2 str vs ogórek.ByteString.
// In this mode decoding/encoding and encoding/decoding operations are always
// identity with respect to strings:
//
// py2 unicode / py3 str ↔ string StrictUnicode=y mode
// py2 str ↔ ogórek.ByteString
//
//
// For bytes, unconditionally to string mode, there is direct 1-1 mapping in
// between Python and Go types:
//
// bytes ↔ ogórek.Bytes (~)
// bytearray ↔ []byte
//
//
//
// Python classes and instances are mapped to Class and Call, for example:
//
// Python Go
......@@ -112,15 +138,28 @@
// stream. For example AsInt64 tries to represent unpickled value as int64 if
// possible and errors if not.
//
// For strings the situation is similar, but a bit different.
// On Python3 strings are unicode strings and binary data is represented by
// bytes type. However on Python2 strings are bytestrings and could contain
// both text and binary data. In the default mode py2 strings, the same way as
// py2 unicode, are decoded into Go strings. However in StrictUnicode mode py2
// strings are decoded into ByteString - the type specially dedicated to
// represent them on Go side. There are two utilities to help programs handle
// all those bytes/string data in the pickle stream in uniform way:
//
// - the program should use AsString if it expects text data -
// either unicode string, or byte string.
// - the program should use AsBytes if it expects binary data -
// either bytes, or byte string.
//
// Using the helpers fits into Python3 strings/bytes model but also allows to
// handle the data generated from under Python2.
//
//
// --------
//
// (*) ogórek is Polish for "pickle".
//
// (+) for Python2 both str and unicode are decoded into string with Python
// str being considered as UTF-8 encoded. Correspondingly for protocol ≤ 2 Go
// string is encoded as UTF-8 encoded Python str, and for protocol ≥ 3 as unicode.
//
// (~) bytes can be produced only by Python3 or zodbpickle (https://pypi.org/project/zodbpickle),
// not by standard Python2. Respectively, for protocol ≤ 2, what ogórek produces
// is unpickled as bytes by Python3 or zodbpickle, and as str by Python2.
......
......@@ -14,7 +14,7 @@ import (
const highestProtocol = 5 // highest protocol version we support generating
// unicode is string that always encodes as unicode pickle object.
// (regular string encodes to unicode pickle object only for protocol >= 3)
// (regular string encodes to unicode pickle object only for protocol >= 3 by default)
type unicode string
type TypeError struct {
......@@ -45,6 +45,12 @@ type EncoderConfig struct {
//
// See Ref documentation for more details.
PersistentRef func(obj interface{}) *Ref
// StrictUnicode, when true, requests to always encode Go string
// objects as Python unicode independently of used pickle protocol.
// See StrictUnicode mode documentation in top-level package overview
// for details.
StrictUnicode bool
}
// NewEncoder returns a new Encoder struct with default values
......@@ -120,6 +126,8 @@ func (e *Encoder) encode(rv reflect.Value) error {
return e.encodeUnicode(rv.String())
case Bytes:
return e.encodeBytes(Bytes(rv.String()))
case ByteString:
return e.encodeByteString(rv.String())
default:
return e.encodeString(rv.String())
}
......@@ -302,7 +310,7 @@ func (e *Encoder) encodeBytes(byt Bytes) error {
return e.encodeCall(&Call{
Callable: Class{Module: "_codecs", Name: "encode"},
Args: Tuple{ulatin1, "latin1"},
Args: Tuple{ulatin1, ByteString("latin1")},
})
}
......@@ -329,12 +337,17 @@ func (e *Encoder) encodeByteArray(bv []byte) error {
}
func (e *Encoder) encodeString(s string) error {
// protocol >= 3 -> encode string as unicode object
// (as python3 does)
if e.config.Protocol >= 3 {
// StrictUnicode || protocol >= 3 -> encode string as unicode object as py3 does
if e.config.StrictUnicode || e.config.Protocol >= 3 {
return e.encodeUnicode(s)
// !StrictUnicode && protocol <= 2 -> encode string as bytestr object as py2 does
} else {
return e.encodeByteString(s)
}
}
func (e *Encoder) encodeByteString(s string) error {
l := len(s)
// protocol >= 1 -> BINSTRING*
......
......@@ -9,9 +9,22 @@ import (
)
func Fuzz(data []byte) int {
f1 := fuzz(data, false)
f2 := fuzz(data, true)
f := f1+f2
if f > 1 {
f = 1
}
return f
}
func fuzz(data []byte, strictUnicode bool) int {
// obj = decode(data) - this tests things like stack overflow in Decoder
buf := bytes.NewBuffer(data)
dec := NewDecoder(buf)
dec := NewDecoderWithConfig(buf, &DecoderConfig{
StrictUnicode: strictUnicode,
})
obj, err := dec.Decode()
if err != nil {
return 0
......@@ -25,9 +38,12 @@ func Fuzz(data []byte) int {
// because obj - as we got it as decoding from input - is known not to
// contain arbitrary Go structs.
for proto := 0; proto <= highestProtocol; proto++ {
subj := fmt.Sprintf("strictUnicode %v: protocol %d", strictUnicode, proto)
buf.Reset()
enc := NewEncoderWithConfig(buf, &EncoderConfig{
Protocol: proto,
StrictUnicode: strictUnicode,
})
err = enc.Encode(obj)
if err != nil {
......@@ -46,19 +62,21 @@ func Fuzz(data []byte) int {
// we cannot encode Class (GLOBAL opcode) with \n at proto <= 4
continue
}
panic(fmt.Sprintf("protocol %d: encode error: %s", proto, err))
panic(fmt.Sprintf("%s: encode error: %s", subj, err))
}
encoded := buf.String()
dec = NewDecoder(bytes.NewBufferString(encoded))
dec = NewDecoderWithConfig(bytes.NewBufferString(encoded), &DecoderConfig{
StrictUnicode: strictUnicode,
})
obj2, err := dec.Decode()
if err != nil {
// must succeed, as buf should contain valid pickle from encoder
panic(fmt.Sprintf("protocol %d: decode back error: %s\npickle: %q", proto, err, encoded))
panic(fmt.Sprintf("%s: decode back error: %s\npickle: %q", subj, err, encoded))
}
if !reflect.DeepEqual(obj, obj2) {
panic(fmt.Sprintf("protocol %d: decode·encode != identity:\nhave: %#v\nwant: %#v", proto, obj2, obj))
panic(fmt.Sprintf("%s: decode·encode != identity:\nhave: %#v\nwant: %#v", subj, obj2, obj))
}
}
......
V\u65e5\u672c\u8a9e
.
\ No newline at end of file
......@@ -132,11 +132,19 @@ type Tuple []interface{}
// Bytes represents Python's bytes.
type Bytes string
// make Bytes and unicode to be represented by %#v distinctly from string
// ByteString represents str from Python2 in StrictUnicode mode.
//
// See StrictUnicode mode documentation in top-level package overview for details.
type ByteString string
// make Bytes, ByteString and unicode to be represented by %#v distinctly from string
// (without GoString %#v emits just "..." for all string, Bytes and unicode)
func (v Bytes) GoString() string {
return fmt.Sprintf("%T(%#v)", v, string(v))
}
func (v ByteString) GoString() string {
return fmt.Sprintf("%T(%#v)", v, string(v))
}
func (v unicode) GoString() string {
return fmt.Sprintf("%T(%#v)", v, string(v))
}
......@@ -175,6 +183,12 @@ type DecoderConfig struct {
//
// See Ref documentation for more details.
PersistentLoad func(ref Ref) (interface{}, error)
// StrictUnicode, when true, requests to decode to Go string only
// Python unicode objects. Python2 bytestrings (py2 str type) are
// decoded into ByteString in this mode. See StrictUnicode mode
// documentation in top-level package overview for details.
StrictUnicode bool
}
// NewDecoder constructs a new Decoder which will decode the pickle stream in r.
......@@ -687,7 +701,7 @@ var errCallNotHandled = errors.New("handleCall: call not handled")
func (d *Decoder) handleCall(class Class, argv Tuple) error {
// for protocols <= 2 Python3 encodes bytes as `_codecs.encode(byt.decode('latin1'), 'latin1')`
if class.Module == "_codecs" && class.Name == "encode" &&
len(argv) == 2 && argv[1] == "latin1" {
len(argv) == 2 && stringEQ(argv[1], "latin1") {
// bytes as latin1-decoded unicode
data, err := decodeLatin1Bytes(argv[0])
......@@ -713,7 +727,7 @@ func (d *Decoder) handleCall(class Class, argv Tuple) error {
}
// bytearray(unicode, encoding)
if len(argv) == 2 && argv[1] == "latin-1" {
if len(argv) == 2 && stringEQ(argv[1], "latin-1") {
// bytes as latin1-decode unicode
data, err := decodeLatin1Bytes(argv[0])
if err != nil {
......@@ -728,6 +742,15 @@ func (d *Decoder) handleCall(class Class, argv Tuple) error {
return errCallNotHandled
}
// pushByteString pushes str as either ByteString or string depending on StrictUnicode setting.
func (d *Decoder) pushByteString(str string) {
if d.config.StrictUnicode {
d.push(ByteString(str))
} else {
d.push(str)
}
}
// Push a string
func (d *Decoder) loadString() error {
line, err := d.readLine()
......@@ -758,7 +781,7 @@ func (d *Decoder) loadString() error {
return err
}
d.push(s)
d.pushByteString(s)
return nil
}
......@@ -812,7 +835,7 @@ func (d *Decoder) loadBinString() error {
if err != nil {
return err
}
d.push(d.buf.String())
d.pushByteString(d.buf.String())
return nil
}
......@@ -847,7 +870,7 @@ func (d *Decoder) loadShortBinString() error {
if err != nil {
return err
}
d.push(d.buf.String())
d.pushByteString(d.buf.String())
return nil
}
......
This diff is collapsed.
......@@ -24,3 +24,50 @@ func AsInt64(x interface{}) (int64, error) {
}
return 0, fmt.Errorf("expect int64|long; got %T", x)
}
// AsBytes tries to represent unpickled value as Bytes.
//
// It succeeds only if the value is either Bytes, or ByteString.
// It does not succeed if the value is string or any other type.
//
// ByteString is treated related to Bytes because ByteString represents str
// type from py2 which can contain both string and binary data.
func AsBytes(x interface{}) (Bytes, error) {
switch x := x.(type) {
case Bytes:
return x, nil
case ByteString:
return Bytes(x), nil
}
return "", fmt.Errorf("expect bytes|bytestr; got %T", x)
}
// AsString tries to represent unpickled value as string.
//
// It succeeds only if the value is either string, or ByteString.
// It does not succeed if the value is Bytes or any other type.
//
// ByteString is treated related to string because ByteString represents str
// type from py2 which can contain both string and binary data.
func AsString(x interface{}) (string, error) {
switch x := x.(type) {
case string:
return x, nil
case ByteString:
return string(x), nil
}
return "", fmt.Errorf("expect unicode|bytestr; got %T", x)
}
// stringEQ compares arbitrary x to string y.
//
// It succeeds only if AsString(x) succeeds and string data of x equals to y.
func stringEQ(x interface{}, y string) bool {
s, err := AsString(x)
if err != nil {
return false
}
return s == y
}
......@@ -52,3 +52,62 @@ func TestAsInt64(t *testing.T) {
}
}
func TestAsBytesString(t *testing.T) {
Ebytes := func(x interface{}) error {
return fmt.Errorf("expect bytes|bytestr; got %T", x)
}
Estring := func(x interface{}) error {
return fmt.Errorf("expect unicode|bytestr; got %T", x)
}
const y = true
const n = false
testv := []struct {
in interface{}
bok bool // AsBytes succeeds
sok bool // AsString succeeds
}{
{"мир", n, y},
{Bytes("мир"), y, n},
{ByteString("мир"), y, y},
{1.0, n, n},
{None{}, n, n},
}
for _, tt := range testv {
bout, berr := AsBytes(tt.in)
sout, serr := AsString(tt.in)
sin := ""
xin := reflect.ValueOf(tt.in)
if xin.Kind() == reflect.String {
sin = xin.String()
}
boutOK := Bytes(sin)
var berrOK error
if !tt.bok {
boutOK = ""
berrOK = Ebytes(tt.in)
}
soutOK := sin
var serrOK error
if !tt.sok {
soutOK = ""
serrOK = Estring(tt.in)
}
if !(bout == boutOK && reflect.DeepEqual(berr, berrOK)) {
t.Errorf("%#v: AsBytes:\nhave %#v %#v\nwant %#v %#v",
tt.in, bout, berr, boutOK, berrOK)
}
if !(sout == soutOK && reflect.DeepEqual(serr, serrOK)) {
t.Errorf("%#v: AsString:\nhave %#v %#v\nwant %#v %#v",
tt.in, sout, serr, soutOK, serrOK)
}
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment