'<' not supported between instances of 'int' and 'str'

[OdifYltsaeb]

If session keys are not all strings, then this line (

django-debug-toolbar/debug_toolbar/panels/request.py

Line 67 in 6a34e00

for k in sorted(request.session.keys())

) generate a TypeError:

TypeError at /myurl

'<' not supported between instances of 'int' and 'str'

because:

>>> l = ['a', 1, 'x', 22]
>>> sorted(l)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '<' not supported between instances of 'int' and 'str'

[matthiask]

How can session keys NOT all be strings? Are you using the PickleSerializer by chance? It is deprecated and will be removed in Django 5.0.

[OdifYltsaeb]

Not sure. Going through someone else's code, and Yes I know that the Django session guidelines say they should be all strings.. but apparently, someone found a way to bypass it :)

So I don't know... perhaps it makes sense to ignore this report, because it appears someone is trying to use Django the way it is not supposed to be used.

[matthiask]

Thank you!

django-debug-toolbar shouldn't crash anyway, but this makes it a normal priority issue for me. @tim-schilling Maybe something for the Djangocon sprints as well, if nobody gets to it earlier?

[tim-schilling]

I'm thinking we sort the session by strings, then anything else goes in at a random order. I think trying to stringify the keys could have averse effects. For example, if someone put a model as the key for the session, it would then call the str function which could be accessing other properties, generating DB requests.

[matthiask]

I think sorting is fine and helpful if it works. Otherwise we should just bail out and show keys and values in the order they appear in the dict.

[ritiksoni00]

Hy, is anybody working on this issue? @tim-schilling if not then id like to work on it

[matthiask]

@ritiksoni00 The djangocon US sprints will take place in October, see https://2022.djangocon.us/sprints/

AFAIK nobody is working on this issue currently. Feel free to jump in!

[ritiksoni00]

Thanks, Matthias for pointing this out but I already got the virtual(😞) ticket for the conf.

for this task according to my understanding, I need to convert all the items into str right? or I understand it wrong.

[matthiask]

My comment from above still applies :) I think the code should just bail out and skip sorting if it fails.

[ritiksoni00]

i try to understand it and what tim says converting them into str can make DB queries for no reason. and according to your comment

i should just use try/except block for handling the error smt like this? Please correct me if I'm wrong.

try:
    (k, request.session.get(k))
    for k in sorted(request.session.keys())
except TypeError:
     pass

edit:1 or smt like this

try:
    (k, request.session.get(k))
    for k in sorted(request.session.keys())
except TypeError:
     (k, request.session.get(k))
     for k in request.session.keys()

[matthiask]

Yes, something like this. Please also spend some time constructing a test so that we do not regress in this area.

[ritiksoni00]

i tried to run the project on codespace and this is how my INTERNAL_IPS looks like

INTERNAL_IPS = ["127.0.0.1", "::1", 'https://ritiksoni00-django-debug-toolbar-5x5q59v6p5jcp4xw.github.dev' , 'https://ritiksoni00-django-debug-toolbar-5x5q59v6p5jcp4xw-8000.githubpreview.dev']

but it still fails and says :-

    Origin checking failed - https://ritiksoni00-django-debug-toolbar-5x5q59v6p5jcp4xw-8000.githubpreview.dev does not match any trusted origins.

what could be the issue? its in the /workspaces/django-debug-toolbar/example/settings.py

[tim-schilling]

@ritiksoni00 I believe that's unrelated to the toolbar. That's part of Django's CSRF protection. This is likely the setting you need to change.

[ritiksoni00]

        if hasattr(request, "session"):
            try:
                session_list = [(k, request.session.get(k))
                                for k in sorted(request.session.keys())]
            except TypeError:
                session_list = [(k, request.session.get(k))
                                for k in request.session.keys()]
            self.record_stats(
                {
                    "session": {
                        "list": session_list
                    }
                }
            )

and for the test I'm confused about how should i write it

def test_session_list_sorted_or_not(self):
        """
        Test varifies that session_list is sorted if all session key is a string, 
        otherwise, it remains unsorted

        See  https://github.com/jazzband/django-debug-toolbar/issues/1668
        """
        self.request.session = {'list': ['foo','bar', 1]}
        response = self.panel.process_request(self.request)
        self.panel.generate_stats(self.request, response)
        record_stats = self.panel.record_stats

Please guide me if the name for the test function is reasonable. is doc string good? and yeah as i said I don't have any idea how I should write test for that case.

[ritiksoni00]

@tim-schilling i need a lil bit of guidance here. (sorry for the ping) but I don't know how will I proceed ahead from here

[tim-schilling]

@ritiksoni00 a good test here is one that will break the code before the change and pass after the change succeeds. Since the original issue was that the session couldn't sort the keys because they weren't all strings, I'd suspect you'll need to change the session to be:

session = {
    1: "some value",
    'list': ['foo', 'bar', 1],
    (2, 3): "tuple key",
}

If using a session with the above structure breaks the code and then works with your changes, then you should be in the clear. Feel free to open a PR with your changes as well. It's easier for Matthias and I to provide feedback there than in an issue when it comes to actual code.

[tim-schilling]

Rather than:

test varifies that session_list is sorted if all session key is a string, otherwise, it remains unsorted

Try:

Verify the session is sorted when all keys are strings.

But you'll need to include in the test both cases, one where all keys are strings and one where there's at least one key that's not (see previous comment).

[tim-schilling]

Fixed by #1673