Skip to content

Upload: Generate a more elegant random token to distinguish simultaneous uploads #4970

Description

@raxod502

Not using the standard bug report template because this is a comment directly on the source code, and not a report of a user-visible problem in a deployed configuration.

I am using the PhotoPrism REST API to upload photos, and since the API documentation isn't finished yet, I am (as recommended) checking the browser console when uploading photos to see which endpoint to use.

All good so far, I see there is a request like POST https://photos.example.com/api/v1/users/us56eo2vflczhcntsq/upload/zp0dcs. Where does the value zp0dcs come from? I check the frontend and find this:

static generateToken() {
return (Math.random() + 1).toString(36).substring(6);
}

This implementation works, but is deeply perplexing to me. It is generating a floating point number, converting it to base 36, and then taking off the first 6 characters (so, 1.nb7z7ohqqv becomes 7ohqqv). The algorithm here has a number of interesting properties:

  • Usually it generates 6-character tokens.
  • But sometimes it generates 7-character tokens instead, depending on how the floating-point number was displayed.
  • And sometimes it generates 5-character tokens instead, if you got unlucky and the floating-point number was evenly divisible by a power of 36.
  • It can even generate shorter than 5-character tokens, down to and including the empty string, if you get sufficiently unlucky (I think around 1 in 300 million?). If you actually hit this last case then the upload would most likely fail because it wouldn't match the server-side route.

All of this is not a big issue, because the upload path isn't security critical and the failure condition is very rare, but just... why? There are lots of ways of generating random strings in JavaScript and it seems like it would be straightforward to use one that doesn't have such strange edge cases. I would be happy to contribute one if desired.

Metadata

Metadata

Assignees

Labels

frontendRequires experience with HTML/JS/CSStestedChanges have been tested successfully

Type

Projects

Status
Release 🌈

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions