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

WebHost: Some refactors and additional checks when uploading files. #2549

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

ThePhar
Copy link
Member

@ThePhar ThePhar commented Dec 1, 2023

What is this fixing or adding?

Does some refactoring and adds additional checks for files that are uploaded to the WebHost.

Additional checks include:

  • Screening for additional known ROM extensions.
  • Validation that .zip files are proper zip files (e.g. not renamed .rar/.7z files).
  • Checks filenames inside subfolders within zips correctly.
  • Ignores dot files (i.e. starts with .).
  • Prevents nested zips from being uploaded (protection against zip fork-bombs).
  • Allows additional supported known options extensions.

Also tweaks some of the error messages to be a bit more clear and list all known valid option files if files are submitted without them.

How was this tested?

  • Uploaded an archive containing dot files inside a __MACOSX folder.
  • Uploaded a zip containing a .zip file.
  • Uploaded a zip containing no files.
  • Uploaded a zip containing no supported files.
  • Uploaded a pre-generated .zip to /generate and options to /hostGame.
  • Uploaded a .png.
  • Uploaded a .7z file renamed as a .zip file

All of these would have given unclear (or thrown 500) error on current, but do not with these changes.

  • Uploaded a valid file.

This one still succeeds. :P

If this makes graphical changes, please attach screenshots.

Host Game
image
image
image

Generate Game
image
image
image
image

@ThePhar ThePhar added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. is: refactor/cleanup Improvements to code/output readability or organizization. affects: webhost Issues/PRs that touch webhost and may need additional validation. labels Dec 1, 2023
allowed_generation_extensions = (".archipelago", ".zip")


def allowed_options(filename: str) -> bool:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't place it, but these feel wrongly named.

Maybe "is_allowed_options_filename"? Sounds a bit overly verbose though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming was honestly the hardest part of this PR.

@ThePhar ThePhar merged commit 0eefe9e into ArchipelagoMW:main Dec 13, 2023
12 checks passed
@ThePhar ThePhar deleted the webhost_upload_ux_improvements branch December 13, 2023 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: webhost Issues/PRs that touch webhost and may need additional validation. is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. is: refactor/cleanup Improvements to code/output readability or organizization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants