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 specify how the uuid for a file is computed as the new crypto.randomUUID forces the use of https #22

Closed
doberkofler opened this issue Oct 31, 2024 · 7 comments

Comments

@doberkofler
Copy link

In version 7.1.0 a change was made (Use crypto.randomUUID instead of a custom function) that now uses crypto.randomUUID and therefore restricts the use of file uploads by forcing the use of http if not running on localhost.
Although I completely understand why this has been changed, it somehow restricts the usability in distributed test environment.
Is there away to customise this behaviour or else would it be possible to offer some customisation on how to calculate a new uuid?

@NicolasCARPi
Copy link
Owner

Could you use a hook on addedFile event (https://docs.dropzone.dev/configuration/events) and modify the file object so that file.upload.uuid is a function that you set?

Otherwise, I'm open to implement a fallback function if the context is not secure, that would be easier than asking consuming code to work around this issue.

@doberkofler
Copy link
Author

doberkofler commented Nov 4, 2024

In my understanding:

  1. The crypto.randomUUID function throws an exception before the addedFile event is triggered as it builds the file object using the crypto.randomUUID function.
  2. The new crypto.randomUUID function seems to have been introduced to restrict uploads to be using https and it seems to me as if a fallback would basically revert this intention.

Maybe I misunderstand the functionality, but it seems to me as if an explicit API that allows to use an alternative to crypto.randomUUID is the only way to keep the effect of using crypto.randomUUID except in specific use cases like unit testing on our case.

@NicolasCARPi
Copy link
Owner

Are you willing to work on a PR for:

for the custom function, you can use this:

https://github.com/dropzone/dropzone/blob/f50d1828ab5df79a76be00d1306cc320e39a27f4/src/dropzone.js#L1721-L1731

@doberkofler
Copy link
Author

I can create a PR but still do not understand if you really just want to have a fallback? It is my understanding that this change was made to prevent uploading without https in the first place and would this fallback not contradict the original intention?

@NicolasCARPi
Copy link
Owner

Initially, this change was made so instead of a custom function, we use the browser's implementation, which results in less line of code for this library, and thus (slightly) enhanced maintainability. That was the original incentive. The fact that secure context is mandatory for this function to run wasn't considered an issue because HTTPS should be default by now, and it considers localhost secure so dev is not impacted.

Using the custom fallback doesn't cause any security issue, it just generates a UUID string, it is not used to encrypt state secrets, so the secure context isn't really needed here. Interestingly, there is a lengthy discussion here about whether randomUUID should require secureContext. Sadly, I don't think this will be changed now anyway :/

So while I don't look forward to adding a branch in the code, if it solves an issue users have, it's worth considering. We could also just remove the randomUUID() and always use the custom function. After all, it didn't cause any issue. It's just unnecessary given that randomUUID() exists now, and that TLS is ubiquitous, with the caveat that it requires secureContext. The performance gain/loss are definitely not a concern here anyway. It's just a matter of using browser's functions instead of rolling our own.

in distributed test environment.

Can you be more verbose? How are you testing things and where does the problem lie exactly?

@doberkofler
Copy link
Author

Thank you for the explanation.
If the main purpose of this change was to standardize code, I agree that a fallback would be the best option and I will prepare a PR accordingly.
Our test environment just uses http (instead of https) to simplify the server configuration and therefore fails.

@doberkofler
Copy link
Author

I just pushed #24

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