SAFE API files sync won't update a file if the new file size is exactly the same as the old file size

Support
1

I’ve found a bug in the way the Files.rs synchronisation works, which I believe is what has been causing unusual bugs for a while (people complaining that FilesContainers aren’t updating or that their NRS is showing “old version”, etc.)

safe-api/files.rs considers three factors when deciding whether or not it should upload a file:

  1. Are we using the force parameter (in which case all files are updated)
  2. Is the file type the same as the previous file type
  3. Is the length of the the file contents the same as the previous length

In scenarios where preprocessors are being used to generate files (for instance, using a bundling agent such as Webpack) it’s very likely that an index.html file will have the same string length after an update (the file names of the linked Javascript files will be the only changed data, and these tend to be the same length, 8-10 random characters. This is traditionally used for “cache punching” when you want CDNs to deliver new versions of files exclusively).

Because the sync test is content agnostic, these kinds of files will never be updated.

This seems like a bug to me, in my opinion the contents of the file (as long as the file is some reasonable size, say under 10MB) should be diffed to see if there have been any substantial changes (or perhaps some sort of file hash).

To combat a likely off-the-cuff response: There is a compare_file_content on the files_map_sync function which is being passed as true, but the value of compare_file_content only dictates error states and checks against the file size, not whether the files contents themselves are actually diffed.

Reproducability

Create a FilesContainer in the CLI, then:

  • Create a file named test.example and give it the content f
  • Sync the file up to the FilesContainer you created earlier without the force flag set

You will get the following success:

FilesContainer synced up (version XXX): "XORURL"
+  ../path/to/test.example  safe://XORURL
  • Update test.example to contain the content b
  • Sync the file up to the FilesContainer you created earlier without the force flag set

You will get the following error:

No changes were required, source location is already in sync with FilesContainer (version XXX) at: "XorURL"

Cheers,
Shane

13 Likes
2

That’s correct @Shane, we have to support this, we didn’t do it before as it would have implied to fetch all files to be able to compare their content.
But now that we are able to calculate the XOR-URL of each individual file, before uploading them, and compare them with XOR-URLs in latest version of the FilesMap, sounds like it could be done and very efficiently without even needing any additional metadata stored on the FilesMap (like the content’s hash), neither fetching the files’ content for comparing them.

8 Likes
3

Awesome, I also noticed that the CLI doesn’t support for force option on safe files sync:

$~/: safe files "path/to/dir" "safe://nrs-name" -r --force
error: Found argument '--force' which wasn't expected, or isn't valid in this context

Normally for something this small I would raise a PR to the API repo but I have a busy afternoon ahead of me so I thought I’d raise it in here since it’s tangentially related.

1 Like
4

This is an old issue, don’t have the link to hand but I hit it updating dWeb Blog.

3 Likes
5

Yes, this issue (which you wrote) :

https://github.com/maidsafe/safe-api/issues/271

1 Like
7

I gave it a quick try and it seems to work just fine, I’ll still have to add automated tests, but in case anyone wants to also give it a try, it compares xorurls when doing a safe files sync: GitHub - bochaco/sn_api at feat-issue-271 (WIP PR: https://github.com/maidsafe/safe-api/pull/424)

6 Likes
8

Yep, this works as I would expect it to and I’m now able to deploy same-length files. Thanks for the quick fix!

5 Likes
9

Nice! ok, I’ll try to get this PR finalised so it’s included in very next release.

Don’t thank me, it’s the power of SAFE :smiley:

5 Likes
10

This topic was automatically closed after 60 days. New replies are no longer allowed.