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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of binary files when using ripgrep scanner #19403

Merged
merged 3 commits into from May 27, 2019

Conversation

Projects
None yet
2 participants
@rafeca
Copy link
Contributor

commented May 24, 2019

This PR is a follow-up of #19348

By pure chance I've found an issue when handling files that have lines with binary content while testing the ripgrep scanner on the gecko-dev repository.

The main problem is found when a file has lines with non-UTF characters it returns a base64 representation of that line in the json output (to not break the JSON formatting). There's more information about it here.

Before this PR, we were not handling this scenario and the parser miserably failed 馃槄

@rafeca rafeca force-pushed the fix-binary-files branch from e365931 to ab90d08 May 24, 2019

@nathansobo
Copy link
Contributor

left a comment

Nice find! 鈿★笍

}

return Buffer.from(input.bytes, 'base64').toString()

This comment has been minimized.

Copy link
@nathansobo

nathansobo May 24, 2019

Contributor

Minor: Putting this in an else block would be more readable to me.

This comment has been minimized.

Copy link
@rafeca

rafeca May 27, 2019

Author Contributor

I've just used a ternary at the end 馃榿

@@ -280,7 +292,7 @@ module.exports = class RipgrepDirectorySearcher {

if (message.type === 'begin') {
pendingEvent = {
filePath: message.data.path.text,
filePath: getText(message.data.path),

This comment has been minimized.

Copy link
@nathansobo

nathansobo May 24, 2019

Contributor

Wow, base64 encoded paths? Awesome.

@nathansobo

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

In case anyone is wondering whether there's anything possible in these JSON objects other than text and bytes, there isn't.

@rafeca rafeca merged commit 752555b into master May 27, 2019

2 checks passed

Atom Pull Requests #20190527.1 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@rafeca rafeca deleted the fix-binary-files branch May 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.