Skip to content

Commit c70d912

Browse files
ByronByron
committed
feat!: blob diff resources and outcomes now inform about the usage of text-conv filters.
This is needed to prevent applications from treating such buffers like user data.
1 parent dda9599 commit c70d912

File tree

3 files changed

+88
-37
lines changed

3 files changed

+88
-37
lines changed

‎gix-diff/src/blob/pipeline.rs‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,13 @@ impl WorktreeRoots {
4242
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug)]
4343
pub enum Data {
4444
/// The data to use for diffing was written into the buffer that was passed during the call to [`Pipeline::convert_to_diffable()`].
45-
Buffer,
45+
Buffer {
46+
/// If `true`, a [binary to text filter](Driver::binary_to_text_command) was used to obtain the buffer,
47+
/// making it a derived value.
48+
///
49+
/// Applications should check for this to avoid treating the buffer content as (original) resource content.
50+
is_derived: bool,
51+
},
4652
/// The size that the binary blob had at the given revision, without having applied filters, as it's either
4753
/// considered binary or above the big-file threshold.
4854
///
@@ -274,7 +280,7 @@ impl Pipeline {
274280
})?;
275281
target.map(|target| {
276282
out.extend_from_slice(gix_path::into_bstr(target).as_ref());
277-
Data::Buffer
283+
Data::Buffer { is_derived: false }
278284
})
279285
} else {
280286
let need_size_only = is_binary == Some(true);
@@ -312,7 +318,7 @@ impl Pipeline {
312318
None
313319
} else {
314320
run_cmd(rela_path, cmd, out)?;
315-
Some(Data::Buffer)
321+
Some(Data::Buffer { is_derived: true })
316322
}
317323
}
318324
None => {
@@ -370,7 +376,7 @@ impl Pipeline {
370376
out.clear();
371377
Data::Binary { size }
372378
} else {
373-
Data::Buffer
379+
Data::Buffer { is_derived: false }
374380
})
375381
}
376382
None => None,
@@ -403,6 +409,7 @@ impl Pipeline {
403409
.try_find(id, out)
404410
.map_err(gix_object::find::existing_object::Error::Find)?
405411
.ok_or_else(|| gix_object::find::existing_object::Error::NotFound { oid: id.to_owned() })?;
412+
let mut is_derived = false;
406413
if matches!(mode, EntryKind::Blob | EntryKind::BlobExecutable)
407414
&& convert == Mode::ToWorktreeAndBinaryToText
408415
|| (convert == Mode::ToGitUnlessBinaryToTextIsPresent
@@ -460,6 +467,7 @@ impl Pipeline {
460467
})?;
461468
out.clear();
462469
run_cmd(rela_path, cmd, out)?;
470+
is_derived = true;
463471
}
464472
None => match res {
465473
ToWorktreeOutcome::Unchanged(_) => {}
@@ -490,7 +498,7 @@ impl Pipeline {
490498
out.clear();
491499
Data::Binary { size }
492500
} else {
493-
Data::Buffer
501+
Data::Buffer { is_derived }
494502
}
495503
};
496504
Some(data)

‎gix-diff/src/blob/platform.rs‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,10 @@ pub mod resource {
101101
Resource {
102102
driver_index: value.conversion.driver_index,
103103
data: value.conversion.data.map_or(Data::Missing, |data| match data {
104-
pipeline::Data::Buffer => Data::Buffer(&value.buffer),
104+
pipeline::Data::Buffer { is_derived } => Data::Buffer {
105+
buf: &value.buffer,
106+
is_derived,
107+
},
105108
pipeline::Data::Binary { size } => Data::Binary { size },
106109
}),
107110
mode: value.mode,
@@ -137,7 +140,15 @@ pub mod resource {
137140
/// The object is missing, either because it didn't exist in the working tree or because its `id` was null.
138141
Missing,
139142
/// The textual data as processed to be in a diffable state.
140-
Buffer(&'a [u8]),
143+
Buffer {
144+
/// The buffer bytes.
145+
buf: &'a [u8],
146+
/// If `true`, a [binary to text filter](super::super::Driver::binary_to_text_command) was used to obtain the buffer,
147+
/// making it a derived value.
148+
///
149+
/// Applications should check for this to avoid treating the buffer content as (original) resource content.
150+
is_derived: bool,
151+
},
141152
/// The size that the binary blob had at the given revision, without having applied filters, as it's either
142153
/// considered binary or above the big-file threshold.
143154
///
@@ -156,10 +167,18 @@ pub mod resource {
156167
/// Return ourselves as slice of bytes if this instance stores data.
157168
pub fn as_slice(&self) -> Option<&'a [u8]> {
158169
match self {
159-
Data::Buffer(d) => Some(d),
170+
Data::Buffer { buf, .. } => Some(buf),
160171
Data::Binary { .. } | Data::Missing => None,
161172
}
162173
}
174+
175+
/// Returns `true` if the data in this instance is derived.
176+
pub fn is_derived(&self) -> bool {
177+
match self {
178+
Data::Missing | Data::Binary { .. } => false,
179+
Data::Buffer { is_derived, .. } => *is_derived,
180+
}
181+
}
163182
}
164183
}
165184

@@ -194,9 +213,8 @@ pub mod set_resource {
194213

195214
///
196215
pub mod prepare_diff {
197-
use bstr::BStr;
198-
199216
use crate::blob::platform::Resource;
217+
use bstr::BStr;
200218

201219
/// The kind of operation that should be performed based on the configuration of the resources involved in the diff.
202220
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
@@ -234,6 +252,11 @@ pub mod prepare_diff {
234252
pub struct Outcome<'a> {
235253
/// The kind of diff that was actually performed. This may include skipping the internal diff as well.
236254
pub operation: Operation<'a>,
255+
/// If `true`, a [binary to text filter](super::super::Driver::binary_to_text_command) was used to obtain the buffer
256+
/// of `old` or `new`, making it a derived value.
257+
///
258+
/// Applications should check for this to avoid treating the buffer content as (original) resource content.
259+
pub old_or_new_is_derived: bool,
237260
/// The old or source of the diff operation.
238261
pub old: Resource<'a>,
239262
/// The new or destination of the diff operation.
@@ -421,7 +444,7 @@ impl Platform {
421444
cmd.args(["/dev/null", ".", "."]);
422445
None
423446
}
424-
resource::Data::Buffer(buf) => {
447+
resource::Data::Buffer { buf, is_derived: _ } => {
425448
let mut tmp = gix_tempfile::new(
426449
std::env::temp_dir(),
427450
gix_tempfile::ContainingDirectory::Exists,
@@ -524,10 +547,15 @@ impl Platform {
524547
.diff_cache
525548
.get(new_key)
526549
.ok_or(prepare_diff::Error::SourceOrDestinationUnset)?;
527-
let mut out = prepare_diff::Outcome {
528-
operation: prepare_diff::Operation::SourceOrDestinationIsBinary,
529-
old: Resource::new(old_key, old),
530-
new: Resource::new(new_key, new),
550+
let mut out = {
551+
let old = Resource::new(old_key, old);
552+
let new = Resource::new(new_key, new);
553+
prepare_diff::Outcome {
554+
operation: prepare_diff::Operation::SourceOrDestinationIsBinary,
555+
old_or_new_is_derived: old.data.is_derived() || new.data.is_derived(),
556+
old,
557+
new,
558+
}
531559
};
532560

533561
match (old.conversion.data, new.conversion.data) {

‎gix-diff/tests/diff/blob/pipeline.rs‎

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ pub(crate) mod convert_to_diffable {
4545
&mut buf,
4646
)?;
4747
assert!(out.driver_index.is_none(), "there was no driver");
48-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
48+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
4949
assert_eq!(buf.as_bstr(), a_content, "there is no transformations configured");
5050

5151
let link_name = "link";
@@ -62,7 +62,7 @@ pub(crate) mod convert_to_diffable {
6262
)?;
6363

6464
assert!(out.driver_index.is_none());
65-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
65+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
6666
assert_eq!(
6767
buf.as_bstr(),
6868
a_name,
@@ -86,7 +86,7 @@ pub(crate) mod convert_to_diffable {
8686
)?;
8787

8888
assert!(out.driver_index.is_none(), "there was no driver");
89-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
89+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
9090
assert_eq!(buf.as_bstr(), b_content, "there is no transformations configured");
9191
}
9292

@@ -203,7 +203,7 @@ pub(crate) mod convert_to_diffable {
203203
assert!(out.driver_index.is_none());
204204
assert_eq!(
205205
out.data,
206-
Some(pipeline::Data::Buffer),
206+
Some(pipeline::Data::Buffer { is_derived: false }),
207207
"links are always read and never considered large"
208208
);
209209
assert_eq!(buf.as_bstr(), large_content);
@@ -340,7 +340,7 @@ pub(crate) mod convert_to_diffable {
340340
&mut buf,
341341
)?;
342342
assert!(out.driver_index.is_none(), "there was no driver");
343-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
343+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
344344
assert_eq!(
345345
buf.as_bstr(),
346346
a_content,
@@ -361,7 +361,7 @@ pub(crate) mod convert_to_diffable {
361361
&mut buf,
362362
)?;
363363
assert!(out.driver_index.is_none(), "there was no driver");
364-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
364+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
365365
assert_eq!(
366366
buf.as_bstr(),
367367
"a\nb",
@@ -386,7 +386,7 @@ pub(crate) mod convert_to_diffable {
386386
)?;
387387

388388
assert!(out.driver_index.is_none());
389-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
389+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
390390
assert_eq!(
391391
buf.as_bstr(),
392392
link_content,
@@ -411,7 +411,7 @@ pub(crate) mod convert_to_diffable {
411411
)?;
412412

413413
assert!(out.driver_index.is_none(), "there was no driver");
414-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
414+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
415415
assert_eq!(buf.as_bstr(), "b-content\r\n", "LF to CRLF by worktree filtering");
416416

417417
let mut db = ObjectDb::default();
@@ -429,7 +429,7 @@ pub(crate) mod convert_to_diffable {
429429
)?;
430430

431431
assert!(out.driver_index.is_none(), "there was no driver");
432-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
432+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
433433
assert_eq!(buf.as_bstr(), "b\n", "no filtering was performed at all");
434434

435435
Ok(())
@@ -524,11 +524,11 @@ pub(crate) mod convert_to_diffable {
524524
)?;
525525

526526
assert_eq!(out.driver_index, Some(0));
527-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
527+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: true }));
528528
assert_eq!(
529529
buf.as_bstr(),
530530
"\0b",
531-
"if binary-text-conversion is set, we don't care if it outputs zero bytes, let everything pass"
531+
"if binary-text-conversion is set, we don't care if it outputs null-bytes, let everything pass"
532532
);
533533

534534
Ok(())
@@ -613,7 +613,12 @@ pub(crate) mod convert_to_diffable {
613613
&mut buf,
614614
)?;
615615
assert_eq!(out.driver_index, Some(0));
616-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
616+
assert_eq!(
617+
out.data,
618+
Some(pipeline::Data::Buffer {
619+
is_derived: !matches!(mode, pipeline::Mode::ToGit)
620+
})
621+
);
617622
assert_eq!(buf.as_bstr(), "to-text\na\n", "filter was applied");
618623
}
619624

@@ -630,7 +635,7 @@ pub(crate) mod convert_to_diffable {
630635
&mut buf,
631636
)?;
632637
assert_eq!(out.driver_index, Some(0));
633-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
638+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
634639
assert_eq!(buf.as_bstr(), "a\n", "unconditionally use git according to mode");
635640

636641
let id = db.insert("a\n");
@@ -648,7 +653,7 @@ pub(crate) mod convert_to_diffable {
648653
&mut buf,
649654
)?;
650655
assert_eq!(out.driver_index, Some(0));
651-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
656+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: true }));
652657
assert_eq!(buf.as_bstr(), "to-text\na\n", "filter was applied");
653658
}
654659

@@ -665,7 +670,7 @@ pub(crate) mod convert_to_diffable {
665670
&mut buf,
666671
)?;
667672
assert_eq!(out.driver_index, Some(0));
668-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
673+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
669674
assert_eq!(
670675
buf.as_bstr(),
671676
"a\n",
@@ -723,7 +728,7 @@ pub(crate) mod convert_to_diffable {
723728
&mut buf,
724729
)?;
725730
assert_eq!(out.driver_index, Some(4), "despite missing, we get driver information");
726-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
731+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
727732
assert_eq!(
728733
buf.as_bstr(),
729734
"link-target",
@@ -796,7 +801,12 @@ pub(crate) mod convert_to_diffable {
796801
&mut buf,
797802
)?;
798803
assert_eq!(out.driver_index, Some(2));
799-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
804+
assert_eq!(
805+
out.data,
806+
Some(pipeline::Data::Buffer {
807+
is_derived: !matches!(mode, pipeline::Mode::ToGit)
808+
})
809+
);
800810
assert_eq!(
801811
buf.as_bstr(),
802812
"to-text\nc\n",
@@ -819,7 +829,7 @@ pub(crate) mod convert_to_diffable {
819829
&mut buf,
820830
)?;
821831
assert_eq!(out.driver_index, Some(2));
822-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
832+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: true }));
823833
assert_eq!(
824834
buf.as_bstr(),
825835
"to-text\nc\n",
@@ -895,7 +905,12 @@ pub(crate) mod convert_to_diffable {
895905
&mut buf,
896906
)?;
897907
assert_eq!(out.driver_index, Some(3));
898-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
908+
assert_eq!(
909+
out.data,
910+
Some(pipeline::Data::Buffer {
911+
is_derived: !matches!(mode, pipeline::Mode::ToGit)
912+
})
913+
);
899914
assert_eq!(
900915
buf.as_bstr(),
901916
"to-text\nd\n",
@@ -915,7 +930,7 @@ pub(crate) mod convert_to_diffable {
915930
&mut buf,
916931
)?;
917932
assert_eq!(out.driver_index, Some(3));
918-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
933+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: true }));
919934
assert_eq!(
920935
buf.as_bstr(),
921936
"to-text\nd-in-db",
@@ -937,7 +952,7 @@ pub(crate) mod convert_to_diffable {
937952
&mut buf,
938953
)?;
939954
assert_eq!(out.driver_index, None);
940-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
955+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
941956
assert_eq!(
942957
buf.as_bstr(),
943958
"e\n",
@@ -958,7 +973,7 @@ pub(crate) mod convert_to_diffable {
958973
&mut buf,
959974
)?;
960975
assert_eq!(out.driver_index, None);
961-
assert_eq!(out.data, Some(pipeline::Data::Buffer));
976+
assert_eq!(out.data, Some(pipeline::Data::Buffer { is_derived: false }));
962977
assert_eq!(
963978
buf.as_bstr(),
964979
"e-in-db",

0 commit comments

Comments
 (0)