check for embedded NUL char in jl_load (for include) #16233
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
stevengj
added
bug
| @@ -4,6 +4,8 @@ using Base.Test | |||
|
|
|||
| @test @__LINE__ == 5 | |||
|
|
|||
| @test_throws ArgumentError include("test_sourcepath.jl\0") | |||
There was a problem hiding this comment.
Maybe just call Core.include directly here? Otherwise, include_from_node1 is confusing the tests.
| @@ -731,6 +731,7 @@ jl_value_t *jl_parse_eval_all(const char *fname, size_t len, | |||
| fl_free_gc_handles(fl_ctx, 1); | |||
| } | |||
| else { | |||
| assert(memchr(fname, 0, len) == NULL); // was checked already in jl_load | |||
There was a problem hiding this comment.
On release-0.4, jl-parse-file is used inside jl_start_parsing_file which does not take the len as input. Could you propose a backported version, preferably w.r.t. tk/backports-0.4.6 if you want to get this in?
There was a problem hiding this comment.
FWIW, these functions should not take a length argument since they really don't support anything other than NUL terminated strings....
There was a problem hiding this comment.
bump again, would need assistance to resolve the conflict if you want this on release-0.4
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.
This fixes a bug I noticed in #16218, in which
include("foo.jl\0")would erroneously succeed if"foo.jl"existed, because the filename would be silently truncated at the NUL char (similar to #10991).I did an audit of the other functions in
src/*.h, and I couldn't find any other problems of this kind.