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

Core: cast all the settings values so they don't try to get pickled later #4362

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

qwint
Copy link
Contributor

@qwint qwint commented Dec 11, 2024

What is this fixing or adding?

cast all the settings values so they don't try to get pickled later

How was this tested?

ran webhost with forcing the elif len(gen_options) >= app.config["JOB_THRESHOLD"]: line to resolve false to trigger the else clause and didn't see unpickling errors

If this makes graphical changes, please attach screenshots.

@github-actions github-actions bot added affects: webhost Issues/PRs that touch webhost and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Dec 11, 2024
@NewSoupVi
Copy link
Member

Despite me having a "competing" PR open, I think this is also a fine solution to the issue and would be happy for it to be merged instead, or both to be merged

@NewSoupVi NewSoupVi added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label Dec 12, 2024
@Berserker66
Copy link
Member

I'm in favor of this one. Adding more stuff to the unpickler is security relevant, something to look out for. Whereas this avoids that.

@NewSoupVi NewSoupVi merged commit 8d9454e into ArchipelagoMW:main Dec 12, 2024
16 checks passed
@qwint qwint deleted the core_pickled_settings branch December 12, 2024 20:38
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. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants