fix: l'import pcap échoue avec des espaces ou caractères spéciaux dans le chemin#99
fix: l'import pcap échoue avec des espaces ou caractères spéciaux dans le chemin#99apanchoo wants to merge 3 commits into
Conversation
…ar-team#88) Replace Capture::from_file with File::open + Capture::from_raw_fd so that libpcap receives a FILE* instead of a raw path string, bypassing encoding issues in pcap_open_offline for paths containing spaces, accents or special characters.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! Cette pull request corrige un bug critique qui empêchait l'importation de fichiers PCAP lorsque leurs chemins contenaient des espaces ou des caractères spéciaux. La solution refactorise le mécanisme d'ouverture de fichier en tirant parti de la gestion native des chemins de Rust et de l'ouverture basée sur les descripteurs de fichier de Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Ce pull request corrige un problème d'importation de fichiers pcap dont le chemin contient des espaces ou des caractères spéciaux. La solution proposée, qui consiste à ouvrir le fichier avec Rust puis à passer le descripteur de fichier à pcap, est bonne pour les systèmes de type Unix. Cependant, elle introduit une dépendance spécifique à Unix qui empêchera le code de compiler sur d'autres plateformes comme Windows. Mes commentaires suggèrent d'utiliser la compilation conditionnelle pour maintenir la compatibilité multiplateforme tout en appliquant le correctif pour Unix.
| use packet_parser::PacketFlow; | ||
| use pcap::Capture; | ||
| use std::fs::File; | ||
| use std::os::unix::io::IntoRawFd; |
There was a problem hiding this comment.
L'utilisation de std::os::unix::io::IntoRawFd rend ce code spécifique à Unix et ne compilera pas sur d'autres plateformes comme Windows. Pour maintenir la compatibilité multiplateforme, qui semble être un objectif du projet (présence de .exe dans .gitignore), il est préférable d'utiliser la compilation conditionnelle.
| use std::os::unix::io::IntoRawFd; | |
| #[cfg(unix)] | |
| use std::os::unix::io::IntoRawFd; |
| fn open_capture(file_path: &str) -> Result<Capture<pcap::Offline>, CaptureStateError> { | ||
| let file = File::open(file_path).map_err(|e| { | ||
| CaptureStateError::Import(PcapImportError::OpenFileError( | ||
| file_path.to_string(), | ||
| e.to_string(), | ||
| )) | ||
| })?; | ||
| let fd = file.into_raw_fd(); | ||
| // SAFETY: fd est valide et nous en sommes propriétaires | ||
| unsafe { Capture::from_raw_fd(fd) }.map_err(|e| { | ||
| CaptureStateError::Import(PcapImportError::OpenFileError( | ||
| file_path.to_string(), | ||
| e.to_string(), | ||
| )) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Cette implémentation de open_capture est spécifique à Unix à cause de into_raw_fd et from_raw_fd. Cela va causer des erreurs de compilation sur Windows. Je suggère d'utiliser la compilation conditionnelle pour fournir une implémentation pour les systèmes Unix et de conserver l'ancienne méthode pour les autres systèmes (comme Windows). Cela permettra à l'application de continuer à compiler sur toutes les plateformes, même si le bug des chemins d'accès persiste sous Windows en attendant une solution dédiée.
fn open_capture(file_path: &str) -> Result<Capture<pcap::Offline>, CaptureStateError> {
#[cfg(unix)]
{
let file = File::open(file_path).map_err(|e| {
CaptureStateError::Import(PcapImportError::OpenFileError(
file_path.to_string(),
e.to_string(),
))
})?;
let fd = file.into_raw_fd();
// SAFETY: fd est valide et nous en sommes propriétaires. `from_raw_fd` en prend possession.
unsafe { Capture::from_raw_fd(fd) }.map_err(|e| {
CaptureStateError::Import(PcapImportError::OpenFileError(
file_path.to_string(),
e.to_string(),
))
})
}
#[cfg(not(unix))]
{
// Conserve l'ancienne implémentation pour les plateformes non-Unix.
// Le bug des chemins existera toujours sur Windows, mais le code compilera.
Capture::from_file(file_path).map_err(|e| {
CaptureStateError::Import(PcapImportError::OpenFileError(
file_path.to_string(),
e.to_string(),
))
})
}
}Sur Windows, pcap_open_offline utilise l'API narrow string (ANSI) qui ne supporte pas les caractères spéciaux/accentués. On utilise GetShortPathNameW pour obtenir le chemin court 8.3 (ASCII) avant de le passer à libpcap.
2 participants
Fixes #88
Problème
Capture::from_fileappelle.to_str()sur le chemin, qui retourneNonepour les chemins non-UTF-8. Dans ce cas,new_rawpasseNULLàpcap_open_offline, qui lit stdin au lieu du fichier.Même pour les chemins UTF-8 valides,
pcap_open_offline(fonction C) peut mal gérer les chemins contenant des espaces ou des caractères spéciaux selon la locale système.Correction
Ajout d'une fonction
open_capturequi :std::fs::File::open— la gestion native des chemins par Rust fonctionne correctement pour tous les chemins valides (espaces, accents,#,(), etc.)Capture::from_raw_fdqui utilisepcap_fopen_offline(FILE*)au lieu depcap_open_offline(path), contournant tout problème d'encodage dans libpcapTest
Testé avec un fichier nommé
mon capture (été) #2.pcap— l'import fonctionne correctement.