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

Choose file popup integer overflow problem #1335

Closed
ivarsg opened this issue Mar 14, 2023 · 7 comments
Closed

Choose file popup integer overflow problem #1335

ivarsg opened this issue Mar 14, 2023 · 7 comments

Comments

@ivarsg
Copy link
Contributor

ivarsg commented Mar 14, 2023

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
Copy link
Contributor Author

ivarsg commented Jun 5, 2023

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

@fsbraun
Copy link
Member

fsbraun commented Jun 5, 2023

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
Copy link
Member

fsbraun commented Jul 13, 2023

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

@stale
Copy link

stale bot commented Aug 21, 2023

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

@stale stale bot closed this as completed Aug 21, 2023
@ivarsg
Copy link
Contributor Author

ivarsg commented Aug 22, 2023

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
Copy link
Member

fsbraun commented Sep 17, 2023

Hope this does it now: #1422 !

@fsbraun fsbraun reopened this Sep 17, 2023
@ivarsg
Copy link
Contributor Author

ivarsg commented Sep 18, 2023

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

@stale stale bot removed the stale label Sep 18, 2023
@fsbraun fsbraun closed this as completed Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants