better file comparing #142
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MilyMilo
commented
Jan 25, 2024
Comment on lines
+437
to
+451
| # Create a dictionary of remote files in { basename: {"url": "", "location": ""} } format | ||
| def _normalize_remote_files(self, remote_files: List[str]) -> Dict[str, Dict[str, str]]: | ||
| normalized = {} | ||
| for f in remote_files: | ||
| file_parts = f.split("?token=")[0].split("/") | ||
| normalized[file_parts[-1]] = {"url": f, "location": f"{file_parts[-2]}/{file_parts[-1]}"} | ||
|
|
||
| return normalized | ||
|
|
||
| # Create a dictionary of sha1sums in { location: sha1sum } format | ||
| def _get_files_sha1sums(self) -> Dict[str, str]: | ||
| r = self.api.get("/api/v1/files?type=challenge") | ||
| r.raise_for_status() | ||
| return {f["location"]: f.get("sha1sum", None) for f in r.json()["data"]} | ||
|
|
There was a problem hiding this comment.
The diff got a bit messy but this is the first real addition
Comment on lines
502
to
+533
| if "files" not in ignore: | ||
| self._delete_existing_files() | ||
| if challenge.get("files"): | ||
| self._create_files() | ||
| # Get basenames of local files to compare against remote files | ||
| local_files = {f.split("/")[-1]: f for f in self.get("files", [])} | ||
| remote_files = self._normalize_remote_files(remote_challenge.get("files", [])) | ||
|
|
||
| # Delete remote files which are no longer defined locally | ||
| for remote_file in remote_files: | ||
| if remote_file not in local_files: | ||
| self._delete_file(remote_files[remote_file]["location"]) | ||
|
|
||
| # Only check for file changes if there are files to upload | ||
| if local_files: | ||
| sha1sums = self._get_files_sha1sums() | ||
| for local_file_name in local_files: | ||
| # Creating a new file | ||
| if local_file_name not in remote_files: | ||
| self._create_file(self.challenge_directory / local_files[local_file_name]) | ||
| continue | ||
|
|
||
| # Updating an existing file | ||
| # sha1sum is present in CTFd 3.7+, use it instead of always re-uploading the file if possible | ||
| remote_file_sha1sum = sha1sums[remote_files[local_file_name]["location"]] | ||
| if remote_file_sha1sum is not None: | ||
| with open(self.challenge_directory / local_files[local_file_name], "rb") as lf: | ||
| local_file_sha1sum = hash_file(lf) | ||
|
|
||
| if local_file_sha1sum == remote_file_sha1sum: | ||
| continue | ||
|
|
||
| # if sha1sums are not present, or the hashes are different, re-upload the file | ||
| self._delete_file(remote_files[local_file_name]["location"]) | ||
| self._create_file(self.challenge_directory / local_files[local_file_name]) |
There was a problem hiding this comment.
Here's the updated code for comparing the sums with sync
Comment on lines
-729
to
+812
| local_files = {Path(f).name: f for f in challenge.get("files", [])} | ||
| remote_files = {f.split("/")[-1].split("?token=")[0]: f for f in remote_challenge["files"]} | ||
| # Check if files defined in challenge.yml are present | ||
| try: | ||
| self._validate_files() | ||
| local_files = {Path(f).name: f for f in challenge.get("files", [])} | ||
| except InvalidChallengeFile: | ||
| return False | ||
|
|
||
| remote_files = self._normalize_remote_files(remote_challenge["files"]) | ||
| # Check if there are no extra local files | ||
| for local_file in local_files: | ||
| if local_file not in remote_files: | ||
| return False | ||
|
|
||
| sha1sums = self._get_files_sha1sums() | ||
| # Check if all remote files are present locally | ||
| for remote_file in remote_files: | ||
| if remote_file not in local_files: | ||
| for remote_file_name in remote_files: | ||
| if remote_file_name not in local_files: | ||
| return False | ||
|
|
||
| # Check if the remote files are the same as local | ||
| r = self.api.get(remote_files[remote_file]) | ||
| # sha1sum is present in CTFd 3.7+, use it instead of downloading the file if possible | ||
| remote_file_sha1sum = sha1sums[remote_files[remote_file_name]["location"]] | ||
| if remote_file_sha1sum is not None: | ||
| with open(self.challenge_directory / local_files[remote_file_name], "rb") as lf: | ||
| local_file_sha1sum = hash_file(lf) | ||
|
|
||
| if local_file_sha1sum != remote_file_sha1sum: | ||
| return False | ||
|
|
||
| return True | ||
|
|
||
| # If sha1sum is not present, download the file and compare the contents | ||
| r = self.api.get(remote_files[remote_file_name]["url"]) | ||
| r.raise_for_status() | ||
| remote_file_contents = r.content | ||
| local_file_contents = (self.challenge_directory / local_files[remote_file]).read_bytes() | ||
| local_file_contents = (self.challenge_directory / local_files[remote_file_name]).read_bytes() |
There was a problem hiding this comment.
Here are changes for mirror
There was a problem hiding this comment.
And the function has been copied from CTFd to be compatible
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use changes in CTFd 3.7+ (sha1sum for files) to determine whether files should be re-uploaded.
CTFd/CTFd#2451
By comparing the hash we can avoid always deleting and re-uploading files when syncing or verifying challenges.
Also did some housekeeping:
load_installed_challengeshould throw an exception when it fails to load the challenge. This has been always assumed and there are checks before that call, but there's just no reason for it to return None instead of throwing.These changes should be backward compatible, sha1sum has been treated completely optionally, and we'll still use the old behavior if it's not present.