nats.py icon indicating copy to clipboard operation
nats.py copied to clipboard

Duplicate message headers are not supported

Open Kazmirchuk opened this issue 3 years ago • 5 comments
trafficstars

Consider the following code:

import asyncio
import nats

async def run():

    async def handler(msg):
        print(f'Received a message on {msg.subject} : {msg.data.decode()}')
        for k in msg.header:
            print(f'{k} : {msg.header[k]}')

    try:
        nc = await nats.connect(servers='nats://localhost:4222')
        await nc.subscribe(subject='subj', cb=handler)
        print("running...")
    except Exception as err:
        print(err)

if __name__ == '__main__':
    loop = asyncio.get_event_loop()
    loop.run_until_complete(run())
    try:
        loop.run_forever()
    finally:
        loop.close()

And I publish a following message using NATS CLI:

nats pub -H hdr1:val1 -H hdr2:val2 -H hdr2:val3 subj payload

Result:

Received a message on subj : payload
hdr1 : val1
hdr2 : val3

So, the header carrying "val2" was lost.

Kazmirchuk avatar Nov 02 '22 09:11 Kazmirchuk

Ignore my previously deleted reply, you are right, the RFC defines that this could happen:

https://stackoverflow.com/questions/4371328/are-duplicate-http-response-headers-acceptable

This is because the header is returned as a dictionary and could not contain dupplicates:

https://github.com/nats-io/nats.py/blob/48059d5191fc8626e6abc85509c54413319c3585/nats/aio/client.py#L1607

But it's a tricky change to make, because changing this behavior causes a breaking change, because the headers dictionary will have to change.

It could also be done by joining the multiple fields in one, by using a comma separated list. Keep in mind @Kazmirchuk that sending multiple headers like you are doing is not advised by the RFC, if they do not mean the same:

This means that, aside from the well-known exception noted below, a sender MUST NOT generate multiple field lines with the same name in a message (whether in the headers or trailers) or append a field line when a field line of the same name already exists in the message, unless that field's definition allows multiple field line values to be recombined as a comma-separated list

source: https://www.rfc-editor.org/rfc/rfc9110.html#section-5.3-3

Making a change would be easy, but the only change that could be done here is, if there are multiple headers, then follow the RFC and make them into an ordered comma separated list. This is what requests does, for example: https://github.com/psf/requests/issues/1415 .

bvanelli avatar Nov 05 '22 14:11 bvanelli

I'm not developing an application. I'm developing 2 open-source NATS clients, and I'm trying to make them behave as close as possible to the official NATS clients.

Firstly, I don't think that Synadia chose to follow the HTTP RFC to the letter. HTTP headers are case-insensitive, while NATS headers are case-sensitive. I think, the RFC is considered a guide for NATS headers, which is perfectly fine.

Secondly, all official NATS clients that I've looked at, treat the headers as a dict (key -> list of values), and it is clearly visible in the API:

  • nats.go: type Header map[string][]string
  • nats.c: natsMsgHeader_Values returns const char *** values for a given key, and no C programmer will use a triple pointer voluntarily
  • nats.net: string[] GetValues (string name)
  • nats.java: List<String> get(String key)

So, nats.py is a notable exception in this list. So, even if this is not going to be fixed, it might be useful to have the issue open for visibility to other people.

For my own work, I need to decide if it is worth to have a more complex API (key -> list of values) rather then (key->value), or not.

Kazmirchuk avatar Nov 07 '22 09:11 Kazmirchuk

Perhaps an explicit note on the documentation and code comments would be enough then?

bvanelli avatar Nov 07 '22 10:11 bvanelli

nats.py also treats the headers as dictionary like the nats.go client, from the server perspective I think it would hold the opaque payload including the headers but what happens after parsing duplicate headers I think it is undefined.

wallyqs avatar Nov 07 '22 17:11 wallyqs

I encountered the same issue when I needed to read multiple values with the same key set by a Go client. As mentioned above, the Go API suggests that multiple values are supported.

Django has a MultiValueDict for these use cases: https://github.com/django/django/blob/main/django/utils/datastructures.py#L49

Example:

>>> d = MultiValueDict({'name': ['Adrian', 'Simon'], 'position': ['Developer']})
>>> d['name']
'Simon'
>>> d.getlist('name')
['Adrian', 'Simon']

wojas avatar Oct 12 '23 15:10 wojas