Actix session insert should check for string

[miketwenty1]

Expected Behavior

To not store a string as string wrapped in a string. (extra double quotes). Unsure if there's some magic I haven't seen to prevent this.

https://github.com/actix/actix-extras/blob/master/actix-session/src/session.rs#L136-L147
We should check if the value is a string. If it's a string we should not use serde_json::to_string

Current Behavior

Using serde_json::to_string on everything including values of type string

Possible Solution

it seems there may have been uniformity reasons for not allowing types other than strings being stored, but I believe this makes the middleware less useful and more awkward.

Steps to Reproduce (for bugs)

1. using redis middleware insert a session value

Context

Double stringified strings result in all those values needing to be doubly unwrapped which is awkward. Would be nice to let users decide what kind of values they want to store and not wrap everything as string.

Your Environment

• Rust Version (I.e, output of rustc -V):
• Actix-* crate(s) Version:
  Latest stable.

[miketwenty1]

Hey @LukeMathWalker is there any chance you can chime in on this? Perhaps I'm doing something wrong. But everything even integer32's are being inserted as strings, and strings are being stored as string wrapped strings.

Examples in one of the redis sessions:

{
    "y": "0",
    "id": "\"67464c5c-980d-4553-b0a1-0f91898e16aa\"",
    "location": "10",
    "x": "0",
}

Instead what I would like is:

{
    "y": 0,
    "id": "67464c5c-980d-4553-b0a1-0f91898e16aa",
    "location": 10,
    "x": 0,
}

[LukeMathWalker]

A question first: why is this an issue?
Are you trying to do something where this becomes a problem or is it more of a curiosity/optimisation/why not question?

[miketwenty1]

@LukeMathWalker this is more of a curiosity thing / potentially a learning opportunity for me.

I was passing data and deserializing data into structs from the session. I have to do extra serde_json::from_str which seemed hacky to me. Then I was curious if I was doing something wrong, if this is intended, or if this is possibly something that could be changed. I'm interested to know more about this decision.

[miketwenty1]

@LukeMathWalker friendly bump on this in case it slipped through the cracks.

[LukeMathWalker]

This is not something I have time to follow right now.

[miketwenty1]

@LukeMathWalker just curious to hear your thoughts on this.

[kittomfv]

{"actix_identity.user_id":"\"User1\""} I got some the same problem, and I even though can not get value from key
By the way, this code seems to be wrong.

 let id = req.get_identity().expect("panic get identity").id().expect("not found id");

This always get a panic ? I tried many times actix-identity. Any ideas for that ?

[brassy-endomorph]

Bumping this again because any serde types that deserialize into a string will break here because they are double serialized but not double deserialized.

And when you fix this, please write a test to ensure that things like strings, dates, and UUIDs correctly deserialize. This library is fundamentally broken without this support.