In pre-msgpack protocol an 'INVALID_{TID,OID}' was always
decoded as 'None' in NEO/py [1]. But in msgpack protocol
this isn't true anymore. An `INVALID_TID` is now decoded
as an `INVALID_TID`. And this then leads to errors later [2].
We fix this by encoding 'INVALID_{TID,OID}' to NIL on the
wire and by decoding NIL to 'INVALID_{TID,OID}'.

---

[1] https://lab.nexedi.com/nexedi/neoppod/-/blob/6332112cba979dfd29b40fe9f98d097911fde696/neo/lib/protocol.py#L579-583
[2] With SQLite backend we can see the following exception:

Traceback (most recent call last):
  File "/home/levin/neo/neo/tests/functional/__init__.py", line 192, in start
    self.run()
  File "/home/levin/neo/neo/tests/functional/__init__.py", line 288, in run
    getattr(neo.scripts,  self.command).main()
  File "/home/levin/neo/neo/scripts/neostorage.py", line 32, in main
    app.run()
  File "/home/levin/neo/neo/storage/app.py", line 196, in run
    self._run()
  File "/home/levin/neo/neo/storage/app.py", line 227, in _run
    self.doOperation()
  File "/home/levin/neo/neo/storage/app.py", line 301, in doOperation
    poll()
  File "/home/levin/neo/neo/storage/app.py", line 145, in _poll
    self.em.poll(1)
  File "/home/levin/neo/neo/lib/event.py", line 160, in poll
    to_process.process()
  File "/home/levin/neo/neo/lib/connection.py", line 508, in process
    self._handlers.handle(self, self._queue.pop(0))
  File "/home/levin/neo/neo/lib/connection.py", line 93, in handle
    self._handle(connection, packet)
  File "/home/levin/neo/neo/lib/connection.py", line 108, in _handle
    pending[0][1].packetReceived(connection, packet)
  File "/home/levin/neo/neo/lib/handler.py", line 125, in packetReceived
    self.dispatch(*args)
  File "/home/levin/neo/neo/lib/handler.py", line 75, in dispatch
    method(conn, *args, **kw)
  File "/home/levin/neo/neo/storage/handlers/client.py", line 67, in askObject
    o = app.dm.getObject(oid, at, before)
  File "/home/levin/neo/neo/storage/database/manager.py", line 484, in getObject
    before_tid and u64(before_tid))
  File "/home/levin/neo/neo/storage/database/sqlite.py", line 336, in _getObject
    r = q(sql + ' AND tid=?', (partition, oid, tid))
OverflowError: Python int too large to convert to SQLite INTEGER

The data type of IdTime is 'Optional[float]' [1], [2]. Before this patch the
msgpack decoder, encoder and sizer could only proceed 'IdTime' in case its data
type was 'float'. Now they also support an IdTime with value NIL.

Without this patch, NEO/go client tests fail [3].

[1] See here, the fifth argument is IdTime:

    https://lab.nexedi.com/nexedi/neoppod/-/blob/3ddb6663/neo/master/handlers/identification.py#L26-27

    This is found to be 'Optional[float]':

    https://lab.nexedi.com/nexedi/neoppod/-/blob/3ddb6663/neo/tests/protocol#L98

[2] This seems to be true for both, pre-msgpack and post-msgpack
    protocol, because in the pre-msgpack NEO/go there is already this
    note:

    https://lab.nexedi.com/kirr/neo/-/blob/1ad088c8/go/neo/proto/proto.go#L352-357

[3] When running NEO/go client tests without this patch, we can see in the NEO/go log file:

```
I0820 13:05:23.813992 1138432 connect.go:115] C?: talk master(127.0.0.1:29203): dial 127.0.0.1:29203 (MASTER): [
I0820 13:05:23.816456 1138432 connect.go:122] C?: talk master(127.0.0.1:29203): dial 127.0.0.1:29203 (MASTER): dialed ok; requesting identification...
I0820 13:05:23.817337 1138432 connect.go:180] C?: talk master(127.0.0.1:29203): dial 127.0.0.1:29203 (MASTER): identification accepted
I0820 13:05:23.817346 1138432 connect.go:181] C?: talk master(127.0.0.1:29203): dial 127.0.0.1:29203 (MASTER): ]
I0820 13:05:23.817369 1138432 mastered.go:187] C?: talk M1: [
I0820 13:05:23.817401 1138432 mastered.go:195] C?: talk M1: ] (after identification: expect nodeTab: 127.0.0.1:41678 - 127.0.0.1:29203 .0: decode: decode: M: NotifyNodeInformation.IdTime: msgp: attempted to decode type "nil" with method for "float64")
W0820 13:05:23.817429 1138432 mastered.go:127] C?: talk master(127.0.0.1:29203): C?: talk M1: after identification: expect nodeTab: 127.0.0.1:41678 - 127.0.0.1:29203 .0: decode: decode: M: NotifyNodeInformation.IdTime: msgp: attempted to decode type "nil" with method for "float64"
```

In pre-msgpack protocol, the msgcode increment logic is like this:

Notify 	 add +1 to next msgcode
Request  add +0 to next msgcode   (next msg is answer & should therefore be the same)
Answer 	 add +1 to next msgcode

('Answer' msgcode is adjusted by 'AnswerBit')

In post-msgpack protocol, the logic is a bit different:

Notify 	 add +1 to next msgcode
Request  add +0 to next msgcode   (next msg is answer & should therefore be the same)
Answer 	 add +2 to next msgcode

So here we produce gaps after Request/Answer pairs.

The enum order has changed in the migration to the msgpack encoded
protocol.

See here for pre-msgpack order:

https://lab.nexedi.com/nexedi/neoppod/-/blob/6332112cb/neo/lib/protocol.py#L62-141

And here for post-msgpack order:

https://lab.nexedi.com/nexedi/neoppod/-/blob/9d0bf97a1327182ac29e95d65fd9e18742c43d1f/neo/lib/protocol.py#L102-181

As this order defines the encoding of the enum values, this needs to be
strictly followed to be compatible (to follow the protocol declaration).

nexedi/neoppod@9d0bf97a
changed the msgcode increment logic, therefore we have to re-create the
test data.

With the switch to the msgpack encoding, the NEO/py server
now only supports the M encoding.

Not only for performance reasons (at least 3% faster) but also because of
several ugly things in the way packets were defined:
- packet field names, which are only documentary; for roots fields,
  they even just duplicate the packet names
- a lot of repetitions for packet names, and even confusion between the name
  of the packet definition and the name of the actual notify/request packet
- the need to implement field types for anything, like PByte to support new
  compression formats, since PBoolean is not enough

neo/lib/protocol.py is now much smaller.

This fixes the basic NEO handshake in msgpack encoding.

Without this fix NEO/go client test fails with a timeout. In NEO/go log
we can see

```
Log file created at: 2024/08/20 12:50:11
Running on machine: ubuntu
Binary: Built with gc go1.18.1 for linux/amd64
Log line format: [IWEF]mmdd hh:mm:ss.uuuuuu threadid file:line] msg
I0820 12:50:11.300574 1137321 client.go:418] neo: open neo://1@127.0.0.1:23004: [
I0820 12:50:11.301130 1137321 mastered.go:119] C?: talk master(127.0.0.1:23004): [
I0820 12:50:11.301140 1137321 connect.go:115] C?: talk master(127.0.0.1:23004): dial 127.0.0.1:23004 (MASTER): [
I0820 12:50:11.303723 1137321 connect.go:119] C?: talk master(127.0.0.1:23004): dial 127.0.0.1:23004 (MASTER): ] (127.0.0.1:42388 - 127.0.0.1:23004: handshake (client): rx hello reply: unexpected EOF)
W0820 12:50:11.303729 1137321 mastered.go:127] C?: talk master(127.0.0.1:23004): dial 127.0.0.1:23004 (MASTER): 127.0.0.1:42388 - 127.0.0.1:23004: handshake (client): rx hello reply: unexpected EOF
I0820 12:50:12.304076 1137321 connect.go:115] C?: talk master(127.0.0.1:23004): dial 127.0.0.1:23004 (MASTER): [
I0820 12:50:12.306096 1137321 connect.go:119] C?: talk master(127.0.0.1:23004): dial 127.0.0.1:23004 (MASTER): ] (127.0.0.1:42410 - 127.0.0.1:23004: handshake (client): rx hello reply: unexpected EOF)
W0820 12:50:12.306102 1137321 mastered.go:127] C?: talk master(127.0.0.1:23004): dial 127.0.0.1:23004 (MASTER): 127.0.0.1:42410 - 127.0.0.1:23004: handshake (client): rx hello reply: unexpected EOF
I0820 12:50:13.306311 1137321 connect.go:115] C?: talk master(127.0.0.1:23004): dial 127.0.0.1:23004 (MASTER): [
I0820 12:50:13.307818 1137321 connect.go:119] C?: talk master(127.0.0.1:23004): dial 127.0.0.1:23004 (MASTER): ] (127.0.0.1:56834 - 127.0.0.1:23004: handshake (client): rx hello reply: unexpected EOF)
W0820 12:50:13.307824 1137321 mastered.go:127] C?: talk master(127.0.0.1:23004): dial 127.0.0.1:23004 (MASTER): 127.0.0.1:56834 - 127.0.0.1:23004: handshake (client): rx hello reply: unexpected EOF
```

And in NEO/py logs we can see:

```
2024-08-20 12:51:35.5152 DEBUG     M1         Rejecting non-NEO <ServerConnection(nid=None, address=127.0.0.1:35404, handler=IdentificationHandler, fd=18, server) at 7f9417467510>
2024-08-20 12:51:35.5153 DEBUG     M1         connection closed for <ServerConnection(nid=None, address=127.0.0.1:35404, handler=IdentificationHandler, closed, server) at 7f9417467510>
2024-08-20 12:51:36.5161 DEBUG     M1         accepted a connection from 127.0.0.1:35408
2024-08-20 12:51:36.5168 DEBUG     M1         connection completed for <ServerConnection(nid=None, address=127.0.0.1:35408, handler=IdentificationHandler, fd=18, server) at 7f9417467210> (from 127.0.0
```

NEO/py defines its handshake packet here:

https://lab.nexedi.com/nexedi/neoppod/-/blob/9d0bf97a1/neo/lib/protocol.py#L27

It's encoded form can be simply revealed with the help of a python shell:

```python
>>> from neo.lib import protocol
>>> protocol.HANDSHAKE_PACKET
'\x92\xa3NEO\x00'
```

* master:
  go/neo/proto: Update 'Compression' to int to support different compression algorithms

With nexedi/neoppod@fd80cc30 NEO/py added support to encode the compression
algorithm with the 'Compression' parameter. Before this, compression could
only be true (= with compression) or false (= without compression). Now
the absence of compression is encoded with 0. Any other number than 0
encodes a compression algorithm. The mapping is currently:

	1 = zlib

In the future, 2 could mean zstd [1].

[1] https://github.com/facebook/zstd/issues/1134

/reviewed-by @kirr
/reviewed-on !6

With nexedi/neoppod@fd80cc30 NEO/py added support to encode the compression
algorithm with the 'Compression' parameter. Before this, compression could
only be true (= with compression) or false (= without compression). Now
the absence of compression is encoded with 0. Any other number than 0
encodes a compression algorithm. The mapping is currently:

	1 = zlib

In the future, 2 could mean zstd [1].

[1] https://github.com/facebook/zstd/issues/1134

/reviewed-by @kirr
/reviewed-on !6

When a node tries to connect to another node it initially sends a
'RequestIdentification' packet. The other node can either reply with
'AcceptIdentification' or in case of a secondary master with
'NotPrimaryMaster'.

In the second case the message id differs from the initial requests
message id. This makes it difficult in a multi-threaded implementation
to proceed this answer: due to the different msg/connection - id the
multi-threaded implementation tries to proceed this incoming message in
a different thread, while the requesting thread waits forever for its
peers reply.

The most straightforward solution is to use the same connection - id for
both possible answers to the 'RequestIdentification' packet. This
doesn't break given NEO/py implementation and is only a small patch.
A workaround in a multi-threaded implementation on the other hand
seems to be much more complicated and time-consuming. Finally it also makes
sense semantically, because "Message IDs are used to identify response
packets" and in the given context the 'NotPrimaryMaster' *is* the de facto
response of a 'RequestIdentification'.

/suggested-at !2
/proposed-for-review-at nexedi/neoppod!20

* master:
  go/neo/proto: Fix KnownMasterList nesting
  go/neo/proto/RowInfo: Fix representation on the wire

Before this patch, the 'KnownMasterList' field of the 'NotPrimaryMaster'
was expected to be structured in the following way:

	ArrayHeader (KnownMasterList)

	    ArrayHeader (KnownMaster)

		ArrayHeader (Address)

		    Host (string)
		    Port (uint16)

However NEO/py sends the following structure:

	ArrayHeader (KnownMasterList)

	    ArrayHeader (Address)

		Host (string)
		Port (uint16)

This also makes sense, as 'KnownMaster' doesn't need to add another
nesting, because it only includes the address.

This patch amends the NEO/go protocol definition to transparently
represent the nesting as it's send by NEO/py. See also
levin.zimmermann/neoppod@18287612 for a similar issue.

----
kirr: tests currently live only on t branch.

/reviewed-by @kirr
/reviewed-on !6

Some NEO protocol packets have the field 'RowList'. This field contains
information about each row of a partition table. In NEO/go the information
of each row is represented with the 'RowInfo' type [1]. This type is defined
as a struct with the field ‘CellList’. ‘CellList’ is defined as a list of
'CellInfo' [1] (e.g. an entry for each cell).

NEO/go {en,de}codes struct types with ‘genStructHead’ (structures in
golang are encoded as arrays in msgpack) [2]. From the 'RowList'
definition, the msgpack decoder currently expects the following msgpack
array structure:

    ArrayHeader (RowList)

        ArrayHeader (RowInfo)

            ArrayHeader (CellList)

                ArrayHeader (CellInfo)

                    int32 (NID)
                    enum (State)

However NEO/py actually sends:

    ArrayHeader (RowList)

        ArrayHeader (CellList)

            ArrayHeader (CellInfo)

                int32 (NID)
                enum (State)

In other words, currently the NEO/go msgpack encoder expects one more
nesting, which NEO/py doesn’t provide (and which also doesn’t seem to
be necessary as the outer nesting would always only contain one
element). We could adjust the msgpack {en,de}coder to introduce an
exception for the 'RowInfo' type, however as the protocol definition in
'proto.go' aims to transparently reflect the structure of the packets on
the wire, it seems to be more appropriate to fix this straight in the
protocol definition. This is also less error-prone as we don't have to
fix all the different positions of the encoder, decoder & sizer and it's
less code (particularly if 'RowInfo' doesn't stay the only case for such
an issue).

[1] https://lab.nexedi.com/kirr/neo/-/blob/1ad088c8/go/neo/proto/proto.go#L391-394
[2] https://lab.nexedi.com/kirr/neo/-/blob/1ad088c8/go/neo/proto/protogen.go#L1770-1775

--------

kirr: I've applied the following interdiff to the original patch of
levin.zimmermann/neoppod@c93d5dbc :

    --- a/go/neo/neo_test.go
    +++ b/go/neo/neo_test.go
    @@ -100,7 +100,7 @@ func _TestMasterStorage(t0 *tEnv) {
                    PTid:           1,
                    NumReplicas:    0,
                    RowList:        []proto.RowInfo{
    -                       proto.RowInfo{proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
    +                       {proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
                    },
            }))

    @@ -173,7 +173,7 @@ func _TestMasterStorage(t0 *tEnv) {
                    PTid:           1,
                    NumReplicas:    0,
                    RowList:        []proto.RowInfo{
    -                       proto.RowInfo{proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
    +                       {proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
                    },
            }))

    --- a/go/neo/proto/proto_test.go
    +++ b/go/neo/proto/proto_test.go
    @@ -210,9 +210,9 @@ func TestMsgMarshal(t *testing.T) {
                            PTid:        0x0102030405060708,
                            NumReplicas: 34,
                            RowList: []RowInfo{
    -                               {CellInfo{11, UP_TO_DATE}, CellInfo{17, OUT_OF_DATE}},
    -                               {CellInfo{11, FEEDING}},
    -                               {CellInfo{11, CORRUPTED}, CellInfo{15, DISCARDED}, CellInfo{23, UP_TO_DATE}},
    +                               {{11, UP_TO_DATE}, {17, OUT_OF_DATE}},
    +                               {{11, FEEDING}},
    +                               {{11, CORRUPTED}, {15, DISCARDED}, {23, UP_TO_DATE}},
                            },
                            },

    @@ -229,9 +229,9 @@ func TestMsgMarshal(t *testing.T) {
                            hex("cf0102030405060708") +
                            hex("22") +
                            hex("93") +
    -                               hex("92"+"920bd40001"+"9211d40000") +
    -                               hex("91"+"920bd40002") +
    -                               hex("93"+"920bd40003"+"920fd40004"+"9217d40001"),
    +                               hex("92"+"920bd40401"+"9211d40400") +
    +                               hex("91"+"920bd40402") +
    +                               hex("93"+"920bd40403"+"920fd40404"+"9217d40401"),
                    },

                    // map[Oid]struct {Tid,Tid,bool}

for cosmetics and because the tests were failing as

    --- FAIL: TestMsgMarshal (0.00s)
        proto_test.go:106: M/proto.AnswerPartitionTable: encode result unexpected:
        proto_test.go:107:  have: 93cf0102030405060708229392920bd404019211d4040091920bd4040293920bd40403920fd404049217d40401
        proto_test.go:108:  want: 93cf0102030405060708229392920bd400019211d4000091920bd4000293920bd40003920fd400049217d40001

/reviewed-by @kirr
/reviewed-on !6

Before this patch, the 'KnownMasterList' field of the 'NotPrimaryMaster'
was expected to be structured in the following way:

	ArrayHeader (KnownMasterList)

	    ArrayHeader (KnownMaster)

		ArrayHeader (Address)

		    Host (string)
		    Port (uint16)

However NEO/py sends the following structure:

	ArrayHeader (KnownMasterList)

	    ArrayHeader (Address)

		Host (string)
		Port (uint16)

This also makes sense, as 'KnownMaster' doesn't need to add another
nesting, because it only includes the address.

This patch amends the NEO/go protocol definition to transparently
represent the nesting as it's send by NEO/py. See also
levin.zimmermann/neoppod@18287612 for a similar issue.

/reviewed-by @kirr
/reviewed-on !6

Some NEO protocol packets have the field 'RowList'. This field contains
information about each row of a partition table. In NEO/go the information
of each row is represented with the 'RowInfo' type [1]. This type is defined
as a struct with the field ‘CellList’. ‘CellList’ is defined as a list of
'CellInfo' [1] (e.g. an entry for each cell).

NEO/go {en,de}codes struct types with ‘genStructHead’ (structures in
golang are encoded as arrays in msgpack) [2]. From the 'RowList'
definition, the msgpack decoder currently expects the following msgpack
array structure:

    ArrayHeader (RowList)

        ArrayHeader (RowInfo)

            ArrayHeader (CellList)

                ArrayHeader (CellInfo)

                    int32 (NID)
                    enum (State)

However NEO/py actually sends:

    ArrayHeader (RowList)

        ArrayHeader (CellList)

            ArrayHeader (CellInfo)

                int32 (NID)
                enum (State)

In other words, currently the NEO/go msgpack encoder expects one more
nesting, which NEO/py doesn’t provide (and which also doesn’t seem to
be necessary as the outer nesting would always only contain one
element). We could adjust the msgpack {en,de}coder to introduce an
exception for the 'RowInfo' type, however as the protocol definition in
'proto.go' aims to transparently reflect the structure of the packets on
the wire, it seems to be more appropriate to fix this straight in the
protocol definition. This is also less error-prone as we don't have to
fix all the different positions of the encoder, decoder & sizer and it's
less code (particularly if 'RowInfo' doesn't stay the only case for such
an issue).

[1] https://lab.nexedi.com/kirr/neo/-/blob/1ad088c8/go/neo/proto/proto.go#L391-394
[2] https://lab.nexedi.com/kirr/neo/-/blob/1ad088c8/go/neo/proto/protogen.go#L1770-1775

--------

kirr: I've applied the following interdiff to the original patch of
levin.zimmermann/neoppod@c93d5dbc :

    --- a/go/neo/neo_test.go
    +++ b/go/neo/neo_test.go
    @@ -100,7 +100,7 @@ func _TestMasterStorage(t0 *tEnv) {
                    PTid:           1,
                    NumReplicas:    0,
                    RowList:        []proto.RowInfo{
    -                       proto.RowInfo{proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
    +                       {proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
                    },
            }))

    @@ -173,7 +173,7 @@ func _TestMasterStorage(t0 *tEnv) {
                    PTid:           1,
                    NumReplicas:    0,
                    RowList:        []proto.RowInfo{
    -                       proto.RowInfo{proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
    +                       {proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
                    },
            }))

    --- a/go/neo/proto/proto_test.go
    +++ b/go/neo/proto/proto_test.go
    @@ -210,9 +210,9 @@ func TestMsgMarshal(t *testing.T) {
                            PTid:        0x0102030405060708,
                            NumReplicas: 34,
                            RowList: []RowInfo{
    -                               {CellInfo{11, UP_TO_DATE}, CellInfo{17, OUT_OF_DATE}},
    -                               {CellInfo{11, FEEDING}},
    -                               {CellInfo{11, CORRUPTED}, CellInfo{15, DISCARDED}, CellInfo{23, UP_TO_DATE}},
    +                               {{11, UP_TO_DATE}, {17, OUT_OF_DATE}},
    +                               {{11, FEEDING}},
    +                               {{11, CORRUPTED}, {15, DISCARDED}, {23, UP_TO_DATE}},
                            },
                            },

    @@ -229,9 +229,9 @@ func TestMsgMarshal(t *testing.T) {
                            hex("cf0102030405060708") +
                            hex("22") +
                            hex("93") +
    -                               hex("92"+"920bd40001"+"9211d40000") +
    -                               hex("91"+"920bd40002") +
    -                               hex("93"+"920bd40003"+"920fd40004"+"9217d40001"),
    +                               hex("92"+"920bd40401"+"9211d40400") +
    +                               hex("91"+"920bd40402") +
    +                               hex("93"+"920bd40403"+"920fd40404"+"9217d40401"),
                    },

                    // map[Oid]struct {Tid,Tid,bool}

for cosmetics and because the tests were failing as

    --- FAIL: TestMsgMarshal (0.00s)
        proto_test.go:106: M/proto.AnswerPartitionTable: encode result unexpected:
        proto_test.go:107:  have: 93cf0102030405060708229392920bd404019211d4040091920bd4040293920bd40403920fd404049217d40401
        proto_test.go:108:  want: 93cf0102030405060708229392920bd400019211d4000091920bd4000293920bd40003920fd400049217d40001

/reviewed-by @kirr
/reviewed-on !6

* master:
  go/neo/neonet: DialLink: Fix SIGSEGV in case client handshake fails
  go/neo/neonet: Demonstrate DialLink misbehaviour when all handshake attempts fail

In case the last 'handshakeClient' call returns an error, 'DialLink'
returns 'link = nil, err = nil'. Callers of 'DialLink' then don't
recognize that 'link' is 'nil', as it's the convention to only check if
'err' is 'nil', which leads to a 'segmentation violation' as soon as
subsequent code tries to access fields of 'link':

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x14 pc=0x7087ae]

goroutine 5 [running]:
lab.nexedi.com/kirr/neo/go/neo/neonet.(*NodeLink).NewConn(0x0)
	/srv/slapgrid/slappart82/srv/runner/instance/slappart6/software_release/parts/wendelin.core/wcfs/neo/go/neo/neonet/connection.go:404 +0x4e
lab.nexedi.com/kirr/neo/go/neo/xneo.Dial.func1()
	/srv/slapgrid/slappart82/srv/runner/instance/slappart6/software_release/parts/wendelin.core/wcfs/neo/go/neo/xneo/connect.go:138 +0x52
lab.nexedi.com/kirr/neo/go/internal/xio.WithCloseOnErrCancel.func2()
	/srv/slapgrid/slappart82/srv/runner/instance/slappart6/software_release/parts/wendelin.core/wcfs/neo/go/internal/xio/xio.go:114 +0x6a
created by lab.nexedi.com/kirr/neo/go/internal/xio.WithCloseOnErrCancel in goroutine 21
	/srv/slapgrid/slappart82/srv/runner/instance/slappart6/software_release/parts/wendelin.core/wcfs/neo/go/internal/xio/xio.go:109 +0x1ad
```

This patch fixes this issue so that now 'err' and 'link' are never both
'nil' again.

/reviewed-by @kirr
/reviewed-on !10

Levin found that when all handshake attempts fail DialLink returns
both link=nil and err=nil which breaks what callers expect and lead to
segmentation fault when accessing that nil link.

-> Add test to demonstrate the problem.

With xfail removed that test currently fails as

    --- FAIL: TestDialLink_AllHandshakeErr (0.00s)
    panic: lab.nexedi.com/kirr/neo/go/neo/neonet.TestDialLink_AllHandshakeErr.gox.func4.1: lab.nexedi.com/kirr/neo/go/neo/neonet.TestDialLink_AllHandshakeErr.func2: DialLink to handshake-rejecting server:
    have: link=<nil> err=<nil>
    want: link=<nil> err=client:1 - server:2: handshake (client): unexpected EOF [recovered]

We will fix the problem in the next patch.

/reported-by @levin.zimmermann
/reported-at !10

I realized a minor mistake I did in writing the tests that needs to
be fixed in order to have reliable test results.

/reviewed-by @kirr
/reviewed-on !9

During our work on 'wendelin.core' URI normalization, Kirill Smelkov
noted a inconsistency between NEO/py and NEO/go URI parser [1]: NEO/py
drops empty query options, while NEO/go preserves them. Let's fix this
inconsistency by adjusting NEO/go to NEO/py behaviour.

[1] nexedi/wendelin.core!28 (comment 212447)

/reviewed-by @kirr
/reviewed-on !9

Hello Kirill,

in nexedi/neoppod!18 and nexedi/neoppod!21 we could find a common solution for a zurl format that previously diverged between NEOgo and NEOpy. The purpose of this MR is to sync again NEOgo and NEOpy zurl format. After merging this, we can continue to sync NEO zurl format in 'wendelin.core' & 'slapos'. Then we finally have unified approach again, which simplifies understanding and reduces unnecessary mental overhead.

As this is strongly related to nexedi/neoppod!21 I thought it'd be a good idea to generally reduce difference and to replace WIP commits with merged NEOpy upstream commits.

Best, Levin

/reviewed-by @kirr
/reviewed-on !7

* lev/sync-zurl:
  client: Don't allow oPtion_nAme in zurl
  app: Remember SSL credentials so that it is possible to retrieve them
  client: Allow to force TLS via neos:// scheme
  client: Don't allow master_nodes and name to be present in options
  Revert "."
  Revert "Y client: Fix URI scheme to move credentials out of query"
  Revert "X Adjust NEO/go to neo:// URL change + py fixups"
  Revert "fixup! Y client: Fix URI scheme to move credentials out of query"
  Revert "Y client: Don't allow master_nodes and name to be present in options"
  go/client/zurl: Sync format to py upstream

Julien notes this is very likely unneeded:
nexedi/neoppod!21 (diffs, comment 195929)

We had it like this since 01a01c8c (client: Add support for zodburi),
but I rechecked zodburi codebase now and it does not do any similar
lowering anywhere.

So drop support for case normalization in zurl options.

/cc @levin.zimmermann
/reviewed-by @jm
/reviewed-on nexedi/neoppod!21

(cherry-picked from commit 798c9f25)

Unfortunately after creating SSL context it is not possible, or at least
I could not find how, to retrieve original credentials with which the
context was created. However wendelin.core needs to be able to take a
client storage, reconstruct zurl to refer to that particular storage,
and pass that zurl to wcfs, so that wcfs, in turn, could access the same
ZODB database.

Given a NEO client instance, it is already possible to retrieve
master_nodes, cluster name, and detect whether SSL is being in use.
However without being able to retrieve original SSL credentials,
reconstructed zurl will not be full and wcfs won't be able to use
exactly the same secrets as python part does.

-> Help wendelin.core by remembering which ca/cert/key were used to
build SSL context.

This information is used by zstor_2zurl in wendelin.core here:

https://lab.nexedi.com/nexedi/wendelin.core/blob/885b3556/lib/zodb.py#L390-418

/cc @levin.zimmermann
/reviewed-by @jm
/reviewed-on nexedi/neoppod!21

Similarly to how it is done with e.g. http:// and https:// - if neos://
is given TLS usage is forced and ca/cert/key must be there either in the
URI itself, or in $NEO_CA, $NEO_CERT and $NEO_KEY environment variables
mimicking the way how e.g. for https:// TLS credentials are taken from
host environment, not from the uri.

The latter might be usability convenience, but is also useful for WCFS
which needs to be able to remove secrets from uri on zurl normalization.

Please see discussion at nexedi/neoppod!18 (comment 184439)
for details.

/cc @levin.zimmermann
/reviewed-by @jm
/reviewed-on nexedi/neoppod!21

(cherry-picked from commit bc3e38ea)

Because list of masters and cluster name must be already present in
netloc and path. Previously e.g.

	neo://db@α,β,γ?master_nodes=a,b,c"

would mean to use master nodes {a,b,c} not {α,β,γ}. Now it is treated as
invalid URI to remove ambiguity. Same for cluster name.

/cc @levin.zimmermann
/reviewed-by @jm
/reviewed-on nexedi/neoppod!21

(cherry-picked from commit 22ccebd6)

This reverts commit a2f192cb. This has
been merged upstream with nexedi/neoppod@17af7f27.
We should rather cherry-pick upstream commit.

This reverts commit b9a42957. In
nexedi/neoppod!18 and
nexedi/neoppod!21 a common
solution for a zurl format was found. This common format keeps
credentials in the query, therefore we should revert patch
b9a42957.

This reverts the py part of 8c974485.
go parts of this patch are handled in 70c0a984e03083ea35678de9ac7599a2364a355d.

This reverts commit cf685fb5. This used
to be a divergence between NEO/py and NEO/go, however in nexedi/neoppod!18 and
nexedi/neoppod!21 a common
solution for a zurl format was found. This common format keeps
credentials in the query, therefore we should revert patch
cf685fb5.

This reverts commit 6047f893 in order to
replace it with py upstream commit nexedi/neoppod@22ccebd6.

NEO/go and NEO/py zurl format diverged over time:

- 8c974485

However with nexedi/neoppod!21 a
common solution was found. From there, this patch aims to adjust NEO/go zurl
format to be in sync with NEO/py zurl format again.

* master:
  go/zodb: Handle common options in zurl in generic layer

/reviewed-by @kirr
/reviewed-on kirr/neo!4

* kirr/t+new-uri:
  Revert "Y client: Adjust URI scheme to move client-specific options to fragment"
  fixup! client.go: Fix URI client option parsing for supported + unsupported options
  client.go: Fix URI client option parsing for supported + unsupported options
  fixup! client_test: Add tests for NEO URI parser
  client_test: Add tests for NEO URI parser
  fixup! client: Refactor openClientByURL for easier testing
  client: Refactor openClientByURL for easier testing
  Y go/zodb: Handle common options in zurl in generic layer

Offload drivers from handling options such as ?read-only=1 and force
them to deal with such options only via DriverOptions, never zurl.

See added comment for details.

/reviewed-by @levin.zimmermann
/reviewed-on !4

This reverts commit 4c9414ea.
This patch was added at a time when nexedi/neoppod!18 wasn't
resolved yet, but we already wanted to proceed with WCFS. Now the NEO MR
is resolved and we decided to mostly leave the NEO zurl as it was
originally implemented in nexedi/neoppod!6.
This means we don't need this patch anymore which changed the NEO
zurl format.

readonly is handled by common zodb.OpenDriver.

Before this patch, the parser ignored options which were already supported
by the client (for instance 'read-only') and even raised an error. But the
client can already use this option: as levin.zimmermann/neoppod@a9246333 describes this should happen in the
local storage URL parser.

Furthermore not-yet-supported client options (for instance compress) broke
the NEO client before this patch. Now these options only raise a warning which
informs the user that they are ignored. Why? We want to use pre-complete NEO
in real-world projects together with NEO/py clusters. Those real-world projects
may already specify options which aren't supported by our NEO/go client yet.
But it doesn't matters so much, because those options are mostly
relevant for other NEO/py cluster clients (e.g. zope nodes). Instead of
filtering those parameters before parsing them to NEO/go in a higher
level (e.g. SlapOS), NEO/go should already support any valid NEO URL
and raise warnings for not yet implemented features.