django-cms / django-filer

File and Image Management Application for django
https://django-filer.readthedocs.io/
Other
1.73k stars 574 forks source link

Choose file popup integer overflow problem #1335

Closed ivarsg closed 9 months ago

ivarsg commented 1 year ago

If the primary key value of the underlying models (File, Image) exceeds JavaScript Number.MAX_SAFE_INTEGER value, the Choose file popup fails to set correct value of the chosen file. As a consequence, on the best scenario popup calling form validation and save fails with 'Select a valid choice. That choice is not one of the available choices.', but in the worst case - form will save successfully with different (!!) value than was chosen (in case if this different value exists in DB as a PK value).

We had the best scenario - instead of chosen PK value '847536516581326850' it tried to save '847536516581326800' (last two digits set to '0') which did not exist in our DB and therefore validation failed.

To fix the problem, it is necessary to patch all the templates, where dismissRelatedImageLookupPopup() JavaScript function is called and enclose {{ file.id|unlocalize }} in single quotes. Thus the value passed to function will be treated as string and not integer literal which is limited to Number.MAX_SAFE_INTEGER.

As a test for solution and workaround until the bugfix, we just overloaded the 'templates/admin/filer/folder/directory_table.html' (we are on 2.2.4, however in master branch there are more templates where fix should be applied) in our application and applied the fix there.

ivarsg commented 1 year ago

Hi, just noticed - the same problem is regarding dismissRelatedFolderLookupPopup() JavaScript function.

fsbraun commented 1 year ago

Hi @ivarsg ! Thanks for bringing this issue to our attention! I have created a PR (#1350) that should fix it. Can you support and test if it works for you?

Just install (on your test system) pip install git+https://github.com/fsbraun/django-filer@fix/unlocalize_ids and test if it works?

I'd appreciate your feedback! Thank you!

fsbraun commented 12 months ago

@ivarsg Can this issue be closed since the fix has been released in version 3.0?

stale[bot] commented 10 months ago

This will now be closed due to inactivity, but feel free to reopen it.

ivarsg commented 10 months ago

Hi @fsbraun,

I'am sorry for so much delayed response, I was a bit overloaded in other projects+vaccation and failed to notice mentions above!

So, I have just now installed normally django-filer without specific versions/source constraints/indications, and I have got version 3.0.4 installed.

The issue is partially fixed - it works correctly on one of the three clickable links and fails on the other two. Here is why: it has been fixed on source code line no 95 (I look at the source code tagged with v3.0.4), but was left unfixed on lines no 103 and 118.

The same applies to templates/admin/filer/folder/directory_thumbnail_list.html (here the respective lines are 94 (fixed), 103, 118 (unfixed)).

Currently the workaround is to click specifically on 'arrow' to chose the file correctly!

fsbraun commented 9 months ago

Hope this does it now: https://github.com/django-cms/django-filer/pull/1422 !

ivarsg commented 9 months ago

@fsbraun I have just tested #1422 by installing it as package in my env, and it fixes Javascript int overflow in our case.