Devil in the details: File ID javascript limit

[labkode]

@DeepDiver1975 @felixboehm @butonic @PVince81

We have hit an issue that may impact other big ownCloud deployments.

ownCloud relies on a file id to be an integer, currently the size has been increased server-side to unsigned 64 bits: #26901

The problem is that Javascript only support numerical precision up to 2^53.

Long story short: the fileInfo model in JS relies on the fileId to be a number and it calls parseInt("file id") but if the number is bigger than 2^53 results in an erratic conversion, see image:

Result: every server side application relying on the file Id sent from the browser will fail as the file id is not correct and can point to an non-existing file or to another file in other user home directory.

The solution for this problem is to make the file ID a string and not a number, the sync client took this sage decision long time ago.

Cheers,
Hugo

[DeepDiver1975]

Yet another t-shirt? 😆

[PVince81]

Is this just a test case or a real life use case ?

It is known that JS has issues with bigger numbers.

Now for the file id I guess we could switch the web UI to use strings everywhere. Not sure about other numbers though where arithmetic is needed (ex: sizes)

[labkode]

@PVince81 real case we hit yesterday.

The first action I took to mitigate it was to change the file id to string in the JS fileInfo model, but that will break at least sharing.

Explanation:
The file id is currently stored in <tr data-id=123 ... >
When changing the fileInfo model to string, it gets converted to:
<tr data-id="123" ... > but when attaching share info from the server (item_source) to the row, the check is done also on the type, so no share info is shown.

Unfortunately, I think the change has to be done also on server side: all responses sending a item_source, file_source, file_id need to be converted to string

[DeepDiver1975]

Unfortunately, I think the change has to be done also on server side: all responses sending a item_source, file_source, file_id needs to be converted to string

json has no understanding of types - values are all strings. Fixing this in pure js side should do the job from my understanding

[PVince81]

I'm already looking into this. I see some additional unit tests failing.

In general I think this is a likely dangerous change as it will affect apps that expect the file id to be an int.

[PVince81]

You might be right about sharing: the sharing code also stores the file id in "item_source" and "file_source" as int, so this will need additional changes there.

[labkode]

@DeepDiver1975 unfortunately, JSON has notion of types, being Number one of them:
See here: https://www.json.org/

@PVince81 I agree is a dangerous change, but I think is a change that needs to be introduced to support big deployments, else the software will not be able to support the needs and hacks will be needed to keep it running.

[DeepDiver1975]

@DeepDiver1975 unfortunately, JSON has notion of types, being Number one of them:
See here: https://www.json.org/

hmmm - so we send this as

'file_id': 12345

and is interpreted as int then - too bad

I had this different in mind - thx for the enlightenment

[PVince81]

@DeepDiver1975 for the parts that parse Webdav it can already work as what we get is strings.

The part we need to look at is when returning file id as int from JSON objects.

Going to look at the sharing code. Will need to change the API code to return "item_source" and "file_source" as strings.

WIP PR here: #30435

[labkode]

@PVince81 thanks for the quick reaction.

In order to not break compatibility you might add just another field:
item_source_string and file_source_string,

This is the solution adopted by Twitter in their public API.

{"id": 10765432100123456789, "id_str": "10765432100123456789", ...}

[PVince81]

hah... https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER

[PVince81]

For my test cases: insert into oc_filecache (fileid) values (12345678901234567); and then delete it again. Any new entries will have higher file id values. Need to upload loads of files to have some trigger the uncertainty.

[individual-it]

ToDo @phil-davis & @individual-it make UI test run using problematic ids

[labkode]

Hi @PVince81 I re-open, as I think there are missing points to cover, specially on the server side.
When browsing the file id is correctly inserted into the address bar, but if you refresh, on the server side, probably the string fileID is not handled correctly. Thx

[stale]

This issue has been automatically closed.