FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Custom search command support for multibyte characters in Python 3 by amysutedja · Pull Request #341 · splunk/splunk-sdk-python · GitHub

Repository navigation

Custom search command support for multibyte characters in Python 3 - #341

Merged
amysutedja merged 8 commits into
masterfrom
csc-multibyte
Sep 9, 2020
Merged

amysutedja merged 8 commits into
masterfrom
csc-multibyte

Conversation

Copy link
Copy Markdown
Contributor

Fixes #288

SearchCommand supports multibyte characters in Python 3

Previously, SearchCommand in Python 3 would read directly from the incoming ifile stream -- typically sys.stdin. In Python 2 sys.stdin is a file-like byte stream, whereas in Python 3 it is an io.TextIOWrapper containing an underlying buffer. Because this object reads by character rather than by byte, multibyte characters would cause the command to read too far past the data's boundary. This could lead to corrupt data reads (if early in the stream) or infinite hangs (if at the end of the stream).

We now retrieve the underlying buffer and read from it when in Python 3. The read bytes are then cast to strings for parsing purposes.

Tests ensure underlying byte stream

Previously, the tests defined a metadata stream with the A character in it (not to be confused with A). In Python 2, this character caused its containing string to become unicode, which caused StringIO to gain that encoding. As a result, the size of the metadata stream was always incorrectly measuring Unicode characters rather than bytes, but under test the read logic would always be handed a Unicode character stream rather than a byte stream.

This has been fixed.

Multibyte test fixture

We now have a new test test_multibyte_chunked which contains a multibyte character test fixture.

amysutedja requested a review from zenmoto September 4, 2020 02:50

Copy link
Copy Markdown

Hah, I've been working since yesterday in a change that is nearly identical to this. Nice fix, if I say so myself =D

I'll retire my fork, this is really good.

aij left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for fixing this so quickly!

I added a couple comments, but nothing worth blocking on.

Copy link
Copy Markdown

FYI we are waiting on this change to upgrade the pythonsdk in all our apps ahead of conf. We'd rather not have to do a monkey patch. Please advise when you can merge and release an official build.

amysutedja merged commit 181cbfe into master Sep 9, 2020
amysutedja deleted the csc-multibyte branch September 9, 2020 18:10
This file contains hidden or 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
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.

StreamingCommand failed when input contains non-ascii character

5 participants


Back | FazBrowse Home | New Git URL