Handle YAML.load_file on Psych 4 #392
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
casperisfine
force-pushed
the
fix-yaml-cache-psych-4-safe-load
branch
5 times, most recently
from
89158d4
to
a95c258
Compare
|
Ok, found an interesting bug: File.write("foo.json", "true")
JSON.load_file("foo.json) # => true
YAML.load_file("foo.json) # EOFErrorIt might seem weird, but since YAML is a superset of JSON, it's not totally invalid, and even if it was we should still fallback properly. Until now it worked because JSON and YAML caches had the same format. But now that YAML caches have a prefix to indicate wether they are safe mode compatible, YAML can't read JSON caches and vice-versa. So each cache would need a key prefix or something. |
casperisfine
force-pushed
the
fix-yaml-cache-psych-4-safe-load
branch
3 times, most recently
from
aa7d24a
to
d767c21
Compare
casperisfine
force-pushed
the
fix-yaml-cache-psych-4-safe-load
branch
from
d767c21
to
f562d2a
Compare
|
The Windows segfault seem consistent and legit. |
casperisfine
force-pushed
the
fix-yaml-cache-psych-4-safe-load
branch
3 times, most recently
from
24af1d4
to
8400bd0
Compare
Context: https://bugs.ruby-lang.org/issues/18492 Rescuing exceptions from a C-extension is complicated, and even seem a bit broken on Windows. It's much simpler to return a special unique value, and likely is more performant too.
casperisfine
force-pushed
the
fix-yaml-cache-psych-4-safe-load
branch
from
7244cef
to
f210df9
Compare
|
Since I ran into https://bugs.ruby-lang.org/issues/18492, I had to refactor further to workaround the issue. Instead of raising |
casperisfine
pushed a commit
to byroot/frozen_record
that referenced
this pull request
Jan 26, 2022
This can cause issues with older versions of Bootsnap Ref: Shopify/bootsnap#392 (comment)
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.
Kind of a fix for #375. Not exactly because it doesn't add support for
safe_load_file, but forYAML.load_fileonpsych 4, which is the most used interface.The idea is that we store a boolean prefix in the cache. When we precompile we try to parse in safe mode, if it fails we fallback to unsafe parsing.
When loading from cache, we know if
load_fileorunsafe_load_fileis used.unsafe_load_filecan use cache generated in safe mode, butload_filewon't use caches generated viaunsafe_load_file.This is a rough prototype, it misses a few bits and needs a lot more testing.