Prevent data being truncated over persistent connection socket #43885
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
We can't rely on readline(), so send the size of the data first. We can then read that many bytes from the stream on the recieving end.
Qalthos
changed the title
ansibot
added
affects_2.7
ansibot
added
the
needs_revision
This comment has been minimized.
This comment has been minimized.
abadger
reviewed
Aug 9, 2018
| from ansible.module_utils.six.moves import cPickle | ||
|
|
||
|
|
||
| def write_to_socket(fd, obj): |
There was a problem hiding this comment.
Naming wise, I don't think these are sockets so we probably don't want to call it that. (also shuld remove socket from the comments)
ansibot
removed
the
needs_triage
And move `write_to_socket` to connection so that they can share
Qalthos
force-pushed
the
remove-unicode-guillotine
branch
from
c45dd68
to
7d012fe
Compare
ansibot
removed
the
needs_revision
|
This looks good to me now. +1 to merge. Note, however, that we should probably switch to using selectors in the future so that we don't ever hang. Here's some test code that demonstrates using selectors: https://gist.github.com/abadger/3c65c3b2d0a8e776bc2b35cd6b45384f It uses selectors on both sides so that neither writing nor reading will hang. |
alikins
added a commit
that referenced
this pull request
Aug 10, 2018
* devel: (30 commits) Prevent data being truncated over persistent connection socket (#43885) Fix eos_command integration test failures (#43922) Update iosxr cliconf plugin (#43837) win_domain modules: ensure Netlogon service is still running after promotion (#43703) openvswitch_db : Handle column value conversion and idempotency in no_key case (#43869) Fix typo Fix spelling of ansbile to ansible (#43898) added platform guides for NOS and VOSS (#43854) Fix download URL for yum integration test. New module for managing EMC VNX Block storage (#42945) Docker integration tests: factorize setup (#42306) VMware: datastore selection (#35812) Remove unnecessary features from cli_command (#43829) [doc] import_role: mention version from which behavior changed and fix some typos (#43843) Add source interface and use-vrf features (#43418) Fix unreferenced msg from vmware_host (#43872) set supports_generate_diff to False vyos (#43873) add group_by_os_family in azure dynamic inventory (#40702) ansible-test: Create public key creating Windows targets (#43760) azure_rm_loadbalancer_facts.py: list() takes at least 2 arguments fix (#29046) (#29050) ...
Qalthos
added a commit
to Qalthos/ansible
that referenced
this pull request
Aug 14, 2018
…le#43885) * Change how data is sent to the persistent connection socket. We can't rely on readline(), so send the size of the data first. We can then read that many bytes from the stream on the recieving end. * Set pty to noncanonical mode before sending * Now that we send data length, we don't need a sentinel anymore * Copy socket changes to persistent, too * Use os.write instead of fdopen()ing and using that. * Follow pickle with sha1sum of pickle * Swap order of vars and init being passed to ansible-connection (cherry picked from commit f221105)
mattclay
pushed a commit
that referenced
this pull request
Aug 14, 2018
* Change how data is sent to the persistent connection socket. We can't rely on readline(), so send the size of the data first. We can then read that many bytes from the stream on the recieving end. * Set pty to noncanonical mode before sending * Now that we send data length, we don't need a sentinel anymore * Copy socket changes to persistent, too * Use os.write instead of fdopen()ing and using that. * Follow pickle with sha1sum of pickle * Swap order of vars and init being passed to ansible-connection (cherry picked from commit f221105)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
SUMMARY
Fixes #43748
ISSUE TYPE
COMPONENT NAME
ansible-connection
ANSIBLE VERSION