Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to configure msgpack serialization and deserialization #287

Open
lunika opened this issue Jan 3, 2022 · 4 comments
Open

Allow to configure msgpack serialization and deserialization #287

lunika opened this issue Jan 3, 2022 · 4 comments

Comments

@lunika
Copy link

lunika commented Jan 3, 2022

Feature Request

Is your feature request related to a problem or unsupported use case? Please describe.

msgpack is able to serialize data and is not able to deserisliaze the data previously serialized. For example if I send this message:

channel_layer = get_channel_layer()
complex_data = {
        "resolutions": {
            144: "video_144_url",
            280: "video_280_url"
        }
    }
    await channel_layer.send(
        "test-channel-1", {"type": "test.message_complex", "complex_data": {
                "resolutions": {
                    144: "video_144_url",
                    280: "video_280_url"
                }
            }
        }
    )

When I will receive it, I will have a msgpack error:

message = await channel_layer.receive("test-channel-1")
###
ValueError: int is not allowed for map key when strict_map_key=True

Describe the solution you'd like

I understand using strict_map_key=False can be a security risk as explain in the msgpack readme but I would like to configure the parameters used by msgpack.packb and msgpack.unpackb or at least don't allow to send a message that can't be deserialized. This configuration can maybe added to the RedisChannelLayer and RedisPubSubChannelLayer __init__ constructor ?

I wrote a failing test to reproduce my case:

@pytest.mark.asyncio
async def test_send_receive_complex_message(channel_layer):
    """
    Makes sure we can send a message to a normal channel then receive it.
    """
    complex_data = {
        "resolutions": {
            144: "video_144_url",
            280: "video_280_url"
        }
    }
    await channel_layer.send(
        "test-channel-1", {"type": "test.message_complex", "complex_data": {
                "resolutions": {
                    144: "video_144_url",
                    280: "video_280_url"
                }
            }
        }
    )
    message = await channel_layer.receive("test-channel-1")
    assert message["type"] == "test.message_complex"
    assert message["complex_data"] == complex_data
@acu192
Copy link
Collaborator

acu192 commented Jan 3, 2022

Both implementations allow you to override serialize() and deserialize(). See this answer for an example.

So, you can call msgpack with other arguments (or even not use msgpack at all).

Does that achieve what you need?

@lunika
Copy link
Author

lunika commented Jan 3, 2022

Hi @acu192 thanks for your quick reply.

It looks like more a workaround than a durable solution. If the RedisChannelLayer.serialize implementation changes, I will not have the benefit of this modification.

@acu192
Copy link
Collaborator

acu192 commented Jan 3, 2022

Yeah the original impl does more stuff in those methods.

On the other hand, the pubsub impl has simpler methods to override.

In either case, this is how you can customize the serialization/de-serialization procedures.

@lunika
Copy link
Author

lunika commented Jan 4, 2022

Thanks I will try to do my own implementation in this case.

lunika added a commit to openfun/marsha that referenced this issue Jan 4, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 5, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 5, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 5, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 5, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 10, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 10, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 11, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 11, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 11, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 11, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
lunika added a commit to openfun/marsha that referenced this issue Jan 11, 2022
The channel layer provided by channel_redis use msgpack to serialize and
deserialize messages. We have a problem when a video is serialized. It
is impossible then to deserialize it because in the urls the resolution
can be used as key and those keys are integer. Msgpack is unable to
deserialize this kind of key. As suggested in the issue[1] open on
channel_redis git repository we implement our own serialize and
deserialize methods.
1: django/channels_redis#287
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants