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

Update C lexer to handle strings with null bytes #5193

Merged
merged 2 commits into from
Dec 18, 2024

Conversation

swalkinshaw
Copy link
Collaborator

StringValueCStr returns a null-terminated char* for a String but a Ruby string might contain null bytes and StringValueCStr will raise an ArgumentError ("string contains null byte").

This switches to using a StringValuePtr to get a (possibly unterminated) char* combined with RSTRING_LEN to get the string's length.

Note: I can't build or test this because I need ragel 7 which I have no idea how to install. But in theory this solution is good? 😅

`StringValueCStr` returns a null-terminated char* for a String but a
Ruby string might contain null bytes and `StringValueCStr` will raise an
`ArgumentError` ("string contains null byte").

This switches to using a `StringValuePtr` to get a (possibly unterminated)
char* combined with `RSTRING_LEN` to get the string's length.
@swalkinshaw swalkinshaw requested a review from rmosolgo December 17, 2024 21:22
@rmosolgo
Copy link
Owner

You can download Ragel directly from here: http://www.colm.net/open-source/ragel/

I think the zip file has instructions for installing 😅 I re-ran it and pushed the changes here, LGTM

@rmosolgo rmosolgo added this to the 2.4.9 milestone Dec 18, 2024
@rmosolgo rmosolgo merged commit ddb420e into master Dec 18, 2024
14 of 15 checks passed
@rmosolgo rmosolgo modified the milestones: 2.4.9, c-parser-1.1.2 Dec 18, 2024
@rmosolgo rmosolgo deleted the c-lexer-handle-strings-with-null-bytes branch December 18, 2024 21:29
@rmosolgo
Copy link
Owner

🚢 in graphql_c-parser 1.1.2, thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants