Jira (PUP-10097) Protect against binary diffs

0 views
Skip to first unread message

Josh Cooper (JIRA)

unread,
Oct 31, 2019, 5:35:04 PM10/31/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
 
Puppet / Bug PUP-10097
Protect against binary diffs
Change By: Josh Cooper
Summary: Using Binary type as a File resource's contents works in puppet apply, but not in puppet agent Protect against binary diffs
Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Josh Cooper (JIRA)

unread,
Oct 31, 2019, 5:42:03 PM10/31/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.4.z
Fix Version/s: PUP 5.5.z

Josh Cooper (JIRA)

unread,
Oct 31, 2019, 5:42:04 PM10/31/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Sprint: Coremunity Grooming

Jarret Lavallee (JIRA)

unread,
Oct 31, 2019, 7:48:03 PM10/31/19
to puppe...@googlegroups.com
Jarret Lavallee updated an issue
Change By: Jarret Lavallee
CS Priority: Needs Priority Reviewed

Henrik Lindberg (JIRA)

unread,
Oct 31, 2019, 9:18:03 PM10/31/19
to puppe...@googlegroups.com
Henrik Lindberg commented on Bug PUP-10097
 
Re: Protect against binary diffs

Shouldn't diff do something more intelligent with binary content? Otherwise it may sometimes return some garbage strange diff that happens to be valid UTF-8. It could for example diff the checksums and return that as text for logging purposes. (Just a thought).

Josh Cooper (JIRA)

unread,
Nov 1, 2019, 5:42:03 PM11/1/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10097

Puppet shells out to the diff executable, and it's implementation specific how it handles diff'ing binary content. In this case, the bytes in the file are classified as:

 file /tmp/test
 
/tmp/test: Non-ISO extended-ASCII text, with no line terminator

Which comes from https://github.com/file/file/blob/954da3a0fb8046e22c6fb74734b39eec79deea1e/src/encoding.c#L142-L146. The only time it considers the file to be binary is if the file contains a character known to never appear in text (like some control chars or 0x7F) https://github.com/file/file/blob/954da3a0fb8046e22c6fb74734b39eec79deea1e/src/encoding.c#L230-L250.

A better approach is probably to use the diff-lcs gem, to detect binary content more gracefully, which would also eliminate issues with diff compatibility across platforms and the lack of diff on windows. I think there's already a ticket filed on that.

Josh Cooper (JIRA)

unread,
Nov 1, 2019, 7:46:03 PM11/1/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Labels: beginner

Josh Cooper (JIRA)

unread,
Nov 11, 2019, 1:43:05 PM11/11/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Team: Coremunity Night's Watch

Josh Cooper (JIRA)

unread,
Nov 11, 2019, 1:43:05 PM11/11/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Sprint: Coremunity Grooming

Mihai Buzgau (JIRA)

unread,
Nov 12, 2019, 10:24:04 AM11/12/19
to puppe...@googlegroups.com
Mihai Buzgau updated an issue
Change By: Mihai Buzgau
Sprint: PR - Triage

Mihai Buzgau (JIRA)

unread,
Nov 14, 2019, 5:51:04 AM11/14/19
to puppe...@googlegroups.com
Mihai Buzgau updated an issue
Change By: Mihai Buzgau
Sprint: PR 2019 - Triage 11-27

Mihai Buzgau (JIRA)

unread,
Nov 14, 2019, 5:51:04 AM11/14/19
to puppe...@googlegroups.com
Mihai Buzgau updated an issue
Change By: Mihai Buzgau
Story Points: 2

Dorin Pleava (JIRA)

unread,
Nov 14, 2019, 8:24:05 AM11/14/19
to puppe...@googlegroups.com
Dorin Pleava assigned an issue to Dorin Pleava
Change By: Dorin Pleava
Assignee: Dorin Pleava

Gheorghe Popescu (JIRA)

unread,
Nov 18, 2019, 7:45:05 AM11/18/19
to puppe...@googlegroups.com
Gheorghe Popescu assigned an issue to Unassigned
Change By: Gheorghe Popescu
Assignee: Dorin Pleava

Ciprian Badescu (JIRA)

unread,
Nov 19, 2019, 3:18:03 AM11/19/19
to puppe...@googlegroups.com
Ciprian Badescu assigned an issue to Ciprian Badescu
Change By: Ciprian Badescu
Assignee: Ciprian Badescu

Ciprian Badescu (JIRA)

unread,
Nov 20, 2019, 8:02:05 AM11/20/19
to puppe...@googlegroups.com
Ciprian Badescu updated an issue
Change By: Ciprian Badescu
Fix Version/s: PUP 5.5.z

Ciprian Badescu (JIRA)

unread,
Nov 20, 2019, 8:07:05 AM11/20/19
to puppe...@googlegroups.com

my understanding is that the change should be implemented on 6.x version, so I removed 5.5.x from fix version/s

Ciprian Badescu (JIRA)

unread,
Nov 20, 2019, 8:18:05 AM11/20/19
to puppe...@googlegroups.com

Josh Cooper, I think using `scrub` could create confusion in some cases, like the bellow example, where I have two different binary files for which diff, with and without scrub, shows no effective change.

I propose to implement your initial proposal, if diff encoding is not valid we should print generic message: `Binary files <A> and <B> differs`

root@chill-menfolk:~# cat /tmp/content_file_test.ckBuSa | od -x
 
0000000 d1c7 85fc
 
0000004
 
root@chill-menfolk:~# cat /tmp/content_file_test.FPDRPp | od -x
 
0000000 d1c7 84fc
 
0000004
 
root@chill-menfolk:~# diff -u /tmp/content_file_test.ckBuSa /tmp/content_file_test.FPDRPp 
--- /tmp/content_file_test.ckBuSa 2019-11-20 11:57:37.651243336 +0000
 
+++ /tmp/content_file_test.FPDRPp 2019-11-20 09:46:00.954412083 +0000
 
@@ -1 +1 @@
 
-ÇÑü
 
\ No newline at end of file
 
+ÇÑü
 
\ No newline at end of file
 
irb(main):002:0> puts `diff -u /tmp/content_file_test.ckBuSa /tmp/content_file_test.FPDRPp`.scrub
 
--- /tmp/content_file_test.ckBuSa	2019-11-20 11:57:37.651243336 +0000
 
+++ /tmp/content_file_test.FPDRPp	2019-11-20 09:46:00.954412083 +0000
 
@@ -1 +1 @@
 
-����
 
\ No newline at end of file
 
+����
 
\ No newline at end of file
 
=> nil

Josh Cooper (JIRA)

unread,
Nov 21, 2019, 12:34:04 PM11/21/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10097

Makes sense Ciprian Badescu

Mihai Buzgau (JIRA)

unread,
Nov 27, 2019, 4:51:10 AM11/27/19
to puppe...@googlegroups.com
Mihai Buzgau updated an issue
Change By: Mihai Buzgau
Sprint: 2019-11-27 , 2019-12-11

Josh Cooper (JIRA)

unread,
Dec 3, 2019, 2:35:04 PM12/3/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.4.z
Fix Version/s: PUP 6.12.0
Fix Version/s: PUP 6.4.5
Fix Version/s: PUP 5.5.18

Josh Cooper (JIRA)

unread,
Dec 3, 2019, 2:42:04 PM12/3/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Release Notes: Not Needed Bug Fix

Josh Cooper (JIRA)

unread,
Dec 3, 2019, 2:44:04 PM12/3/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Release Notes Summary: Previously "puppet agent -t" or "puppet apply --show_diff" could generate an error when trying to display the changes it made to a binary file. Puppet now detects this case and prints a generic message that the binary files differ.

Mihai Buzgau (JIRA)

unread,
Dec 11, 2019, 4:34:06 AM12/11/19
to puppe...@googlegroups.com
Mihai Buzgau updated an issue
Change By: Mihai Buzgau
Sprint: 2019-11-27, 2019-12-11 , 2019-12-24

Kate Medred (JIRA)

unread,
Jan 13, 2020, 5:16:03 PM1/13/20
to puppe...@googlegroups.com
Kate Medred updated an issue
Change By: Kate Medred
Labels: beginner resolved-issue-added
Reply all
Reply to author
Forward
0 new messages