Skip to content

Fix non-strings also being escaped when copied #307

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
encounter merged 2 commits into from
Dec 23, 2025
Merged

Fix non-strings also being escaped when copied #307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd04d49
Fix non-strings also being escaped when copied
LagoLunatic Dec 23, 2025
2445e4f
Move copy escape logic into objdiff-core
LagoLunatic Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 37 additions & 13 deletions objdiff-core/src/arch/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl fmt::Display for DataType {
impl DataType {
pub fn display_labels(&self, endian: object::Endianness, bytes: &[u8]) -> Vec<String> {
let mut strs = Vec::new();
for (literal, label_override) in self.display_literals(endian, bytes) {
for (literal, label_override, _) in self.display_literals(endian, bytes) {
let label = label_override.unwrap_or_else(|| self.to_string());
strs.push(format!("{label}: {literal:?}"))
}
Expand All @@ -94,7 +94,7 @@ impl DataType {
&self,
endian: object::Endianness,
bytes: &[u8],
) -> Vec<(String, Option<String>)> {
) -> Vec<(String, Option<String>, Option<String>)> {
let mut strs = Vec::new();
if self.required_len().is_some_and(|l| bytes.len() < l) {
log::warn!(
Expand All @@ -118,34 +118,34 @@ impl DataType {
match self {
DataType::Int8 => {
let i = i8::from_ne_bytes(bytes.try_into().unwrap());
strs.push((format!("{i:#x}"), None));
strs.push((format!("{i:#x}"), None, None));

if i < 0 {
strs.push((format!("{:#x}", ReallySigned(i)), None));
strs.push((format!("{:#x}", ReallySigned(i)), None, None));
}
}
DataType::Int16 => {
let i = endian.read_i16_bytes(bytes.try_into().unwrap());
strs.push((format!("{i:#x}"), None));
strs.push((format!("{i:#x}"), None, None));

if i < 0 {
strs.push((format!("{:#x}", ReallySigned(i)), None));
strs.push((format!("{:#x}", ReallySigned(i)), None, None));
}
}
DataType::Int32 => {
let i = endian.read_i32_bytes(bytes.try_into().unwrap());
strs.push((format!("{i:#x}"), None));
strs.push((format!("{i:#x}"), None, None));

if i < 0 {
strs.push((format!("{:#x}", ReallySigned(i)), None));
strs.push((format!("{:#x}", ReallySigned(i)), None, None));
}
}
DataType::Int64 => {
let i = endian.read_i64_bytes(bytes.try_into().unwrap());
strs.push((format!("{i:#x}"), None));
strs.push((format!("{i:#x}"), None, None));

if i < 0 {
strs.push((format!("{:#x}", ReallySigned(i)), None));
strs.push((format!("{:#x}", ReallySigned(i)), None, None));
}
}
DataType::Float => {
Expand All @@ -156,6 +156,7 @@ impl DataType {
object::Endianness::Big => f32::from_be_bytes(bytes),
}),
None,
None,
));
}
DataType::Double => {
Expand All @@ -166,24 +167,29 @@ impl DataType {
object::Endianness::Big => f64::from_be_bytes(bytes),
}),
None,
None,
));
}
DataType::Bytes => {
strs.push((format!("{bytes:#?}"), None));
strs.push((format!("{bytes:#?}"), None, None));
}
DataType::String => {
if let Some(nul_idx) = bytes.iter().position(|&c| c == b'\0') {
let str_bytes = &bytes[..nul_idx];
// Special case to display (ASCII) as the label for ASCII-only strings.
let (cow, _, had_errors) = encoding_rs::UTF_8.decode(str_bytes);
if !had_errors && cow.is_ascii() {
strs.push((format!("{cow}"), Some("ASCII".into())));
let string = format!("{cow}");
let copy_string = escape_special_ascii_characters(string.clone());
strs.push((string, Some("ASCII".into()), Some(copy_string)));
}
for (encoding, encoding_name) in SUPPORTED_ENCODINGS {
let (cow, _, had_errors) = encoding.decode(str_bytes);
// Avoid showing ASCII-only strings more than once if the encoding is ASCII-compatible.
if !had_errors && (!encoding.is_ascii_compatible() || !cow.is_ascii()) {
strs.push((format!("{cow}"), Some(encoding_name.into())));
let string = format!("{cow}");
let copy_string = escape_special_ascii_characters(string.clone());
strs.push((string, Some(encoding_name.into()), Some(copy_string)));
}
}
}
Expand Down Expand Up @@ -499,3 +505,21 @@ pub struct RelocationOverride {
pub target: RelocationOverrideTarget,
pub addend: i64,
}

/// Escape ASCII characters such as \n or \t, but not Unicode characters such as \u{3000}.
/// Suitable for copying to clipboard.
fn escape_special_ascii_characters(value: String) -> String {
let mut escaped = String::new();
escaped.push('"');
for c in value.chars() {
if c.is_ascii() {
for e in c.escape_default() {
escaped.push(e);
}
} else {
escaped.push(c);
}
}
escaped.push('"');
escaped
}
8 changes: 6 additions & 2 deletions objdiff-core/src/arch/ppc/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,9 +440,13 @@ impl Arch for ArchPpc {
let simplified = ins.simplified().to_string();
let show_orig = orig != simplified;
let mut out = Vec::with_capacity(2);
out.push(ContextItem::Copy { value: simplified, label: None });
out.push(ContextItem::Copy { value: simplified, label: None, copy_string: None });
if show_orig {
out.push(ContextItem::Copy { value: orig, label: Some("original".to_string()) });
out.push(ContextItem::Copy {
value: orig,
label: Some("original".to_string()),
copy_string: None,
});
}
out
}
Expand Down
36 changes: 25 additions & 11 deletions objdiff-core/src/diff/display.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ impl From<&DiffText<'_>> for HighlightKind {
}

pub enum ContextItem {
Copy { value: String, label: Option<String> },
Copy { value: String, label: Option<String>, copy_string: Option<String> },
Navigate { label: String, symbol_index: usize, kind: SymbolNavigationKind },
Separator,
}
Expand Down Expand Up @@ -398,16 +398,17 @@ pub fn symbol_context(obj: &Object, symbol_index: usize) -> Vec<ContextItem> {
return Vec::new();
};
let mut out = Vec::new();
out.push(ContextItem::Copy { value: symbol.name.clone(), label: None });
out.push(ContextItem::Copy { value: symbol.name.clone(), label: None, copy_string: None });
if let Some(name) = &symbol.demangled_name {
out.push(ContextItem::Copy { value: name.clone(), label: None });
out.push(ContextItem::Copy { value: name.clone(), label: None, copy_string: None });
}
if symbol.section.is_some()
&& let Some(address) = symbol.virtual_address
{
out.push(ContextItem::Copy {
value: format!("{address:x}"),
label: Some("virtual address".to_string()),
copy_string: None,
});
}
out.append(&mut obj.arch.symbol_context(obj, symbol_index));
Expand Down Expand Up @@ -501,8 +502,8 @@ pub fn relocation_context(
let literals = display_ins_data_literals(obj, ins);
if !literals.is_empty() {
out.push(ContextItem::Separator);
for (literal, label_override) in literals {
out.push(ContextItem::Copy { value: literal, label: label_override });
for (literal, label_override, copy_string) in literals {
out.push(ContextItem::Copy { value: literal, label: label_override, copy_string });
}
}
}
Expand Down Expand Up @@ -598,24 +599,37 @@ pub fn instruction_context(
for byte in resolved.code {
hex_string.push_str(&format!("{byte:02x}"));
}
out.push(ContextItem::Copy { value: hex_string, label: Some("instruction bytes".to_string()) });
out.push(ContextItem::Copy {
value: hex_string,
label: Some("instruction bytes".to_string()),
copy_string: None,
});
out.append(&mut obj.arch.instruction_context(obj, resolved));
if let Some(virtual_address) = resolved.symbol.virtual_address {
let offset = resolved.ins_ref.address - resolved.symbol.address;
out.push(ContextItem::Copy {
value: format!("{:x}", virtual_address + offset),
label: Some("virtual address".to_string()),
copy_string: None,
});
}
for arg in &ins.args {
if let InstructionArg::Value(arg) = arg {
out.push(ContextItem::Copy { value: arg.to_string(), label: None });
out.push(ContextItem::Copy { value: arg.to_string(), label: None, copy_string: None });
match arg {
InstructionArgValue::Signed(v) => {
out.push(ContextItem::Copy { value: v.to_string(), label: None });
out.push(ContextItem::Copy {
value: v.to_string(),
label: None,
copy_string: None,
});
}
InstructionArgValue::Unsigned(v) => {
out.push(ContextItem::Copy { value: v.to_string(), label: None });
out.push(ContextItem::Copy {
value: v.to_string(),
label: None,
copy_string: None,
});
}
_ => {}
}
Expand Down Expand Up @@ -677,7 +691,7 @@ pub fn instruction_hover(
let literals = display_ins_data_literals(obj, resolved);
if !literals.is_empty() {
out.push(HoverItem::Separator);
for (literal, label_override) in literals {
for (literal, label_override, _) in literals {
out.push(HoverItem::Text {
label: label_override.unwrap_or_else(|| ty.to_string()),
value: format!("{literal:?}"),
Expand Down Expand Up @@ -871,7 +885,7 @@ pub fn display_ins_data_labels(obj: &Object, resolved: ResolvedInstructionRef) -
pub fn display_ins_data_literals(
obj: &Object,
resolved: ResolvedInstructionRef,
) -> Vec<(String, Option<String>)> {
) -> Vec<(String, Option<String>, Option<String>)> {
let Some(reloc) = resolved.relocation else {
return Vec::new();
};
Expand Down
23 changes: 2 additions & 21 deletions objdiff-gui/src/views/diff.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -865,24 +865,6 @@ pub fn hover_items_ui(ui: &mut Ui, items: Vec<HoverItem>, appearance: &Appearanc
}
}

/// Escape ASCII characters such as \n or \t, but not Unicode characters such as \u{3000}.
/// Suitable for copying to clipboard.
fn escape_special_ascii_characters(value: String) -> String {
let mut escaped = String::new();
escaped.push('"');
for c in value.chars() {
if c.is_ascii() {
for e in c.escape_default() {
escaped.push(e);
}
} else {
escaped.push(c);
}
}
escaped.push('"');
escaped
}

pub fn context_menu_items_ui(
ui: &mut Ui,
items: Vec<ContextItem>,
Expand All @@ -892,7 +874,7 @@ pub fn context_menu_items_ui(
let mut ret = None;
for item in items {
match item {
ContextItem::Copy { value, label } => {
ContextItem::Copy { value, label, copy_string } => {
let mut job = LayoutJob::default();
write_text("Copy ", appearance.text_color, &mut job, appearance.code_font.clone());
write_text(
Expand All @@ -912,8 +894,7 @@ pub fn context_menu_items_ui(
write_text(")", appearance.text_color, &mut job, appearance.code_font.clone());
}
if ui.button(job).clicked() {
let escaped = escape_special_ascii_characters(value);
ui.ctx().copy_text(escaped);
ui.ctx().copy_text(copy_string.unwrap_or(value));
ui.close();
}
}
Expand Down
6 changes: 4 additions & 2 deletions objdiff-wasm/src/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ impl GuestDisplay for Component {
return vec![ContextItem::Copy(ContextItemCopy {
value: "Failed to resolve instruction".to_string(),
label: Some("error".to_string()),
copy_string: None,
})];
};
let ins = match obj.arch.process_instruction(resolved, &diff_config) {
Expand All @@ -281,6 +282,7 @@ impl GuestDisplay for Component {
return vec![ContextItem::Copy(ContextItemCopy {
value: e.to_string(),
label: Some("error".to_string()),
copy_string: None,
})];
}
};
Expand Down Expand Up @@ -707,8 +709,8 @@ impl From<diff::display::HoverItemColor> for HoverItemColor {
impl From<diff::display::ContextItem> for ContextItem {
fn from(item: diff::display::ContextItem) -> Self {
match item {
diff::display::ContextItem::Copy { value, label } => {
ContextItem::Copy(ContextItemCopy { value, label })
diff::display::ContextItem::Copy { value, label, copy_string } => {
ContextItem::Copy(ContextItemCopy { value, label, copy_string })
}
diff::display::ContextItem::Navigate { label, symbol_index, kind } => {
ContextItem::Navigate(ContextItemNavigate {
Expand Down
1 change: 1 addition & 0 deletions objdiff-wasm/wit/objdiff.wit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ interface display {
record context-item-copy {
value: string,
label: option<string>,
copy-string: option<string>,
}

record context-item-navigate {
Expand Down
Loading