From 6ae66cb2bb2a3a4d3bf793b5e7fc6f5d74748993 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 17 Mar 2020 21:11:27 +0100 Subject: [PATCH] nfs: code cleanups Use 'if let' to replace simple 'match' statements. Use explicit returns to easy code review. --- rust/src/nfs/nfs.rs | 32 ++--- rust/src/nfs/nfs2.rs | 13 +- rust/src/nfs/nfs3.rs | 307 +++++++++++++++++-------------------------- rust/src/nfs/nfs4.rs | 36 +++-- 4 files changed, 155 insertions(+), 233 deletions(-) diff --git a/rust/src/nfs/nfs.rs b/rust/src/nfs/nfs.rs index 365f949907..0832d70f0a 100644 --- a/rust/src/nfs/nfs.rs +++ b/rust/src/nfs/nfs.rs @@ -657,15 +657,12 @@ impl NFSState { } let file_handle = w.handle.value.to_vec(); - let file_name = match self.namemap.get(w.handle.value) { - Some(n) => { - SCLogDebug!("WRITE name {:?}", n); - n.to_vec() - }, - None => { - SCLogDebug!("WRITE object {:?} not found", w.handle.value); - Vec::new() - }, + let file_name = if let Some(name) = self.namemap.get(w.handle.value) { + SCLogDebug!("WRITE name {:?}", name); + name.to_vec() + } else { + SCLogDebug!("WRITE object {:?} not found", w.handle.value); + Vec::new() }; let found = match self.get_file_tx_by_handle(&file_handle, STREAM_TOSERVER) { @@ -912,10 +909,8 @@ impl NFSState { if nfs_version == 2 { let size = match parse_nfs2_attribs(reply.attr_blob) { - Ok((_, ref attr)) => { - attr.asize - }, - _ => { 0 }, + Ok((_, ref attr)) => attr.asize, + _ => 0, }; SCLogDebug!("NFSv2 READ reply record: File size {}. Offset {} data len {}: total {}", size, chunk_offset, reply.data_len, chunk_offset + reply.data_len as u64); @@ -1003,13 +998,12 @@ impl NFSState { } fn peek_reply_record(&mut self, r: &RpcPacketHeader) -> u32 { - let xidmap; - match self.requestmap.get(&r.xid) { - Some(p) => { xidmap = p; }, - _ => { SCLogDebug!("REPLY: xid {} NOT FOUND", r.xid); return 0; }, + if let Some(xidmap) = self.requestmap.get(&r.xid) { + return xidmap.procedure; + } else { + SCLogDebug!("REPLY: xid {} NOT FOUND", r.xid); + return 0; } - - xidmap.procedure } pub fn parse_tcp_data_ts_gap<'b>(&mut self, gap_size: u32) -> u32 { diff --git a/rust/src/nfs/nfs2.rs b/rust/src/nfs/nfs2.rs index 86f5de81e8..345d3f018f 100644 --- a/rust/src/nfs/nfs2.rs +++ b/rust/src/nfs/nfs2.rs @@ -91,7 +91,7 @@ impl NFSState { SCLogDebug!("NFSv2: TS creating xidmap {}", r.hdr.xid); self.requestmap.insert(r.hdr.xid, xidmap); - 0 + return 0; } pub fn process_reply_record_v2<'b>(&mut self, r: &RpcReplyPacket<'b>, xidmap: &NFSRequestXidMap) -> u32 { @@ -110,11 +110,9 @@ impl NFSState { }, } } else { - let stat = match be_u32(&r.prog_data) as IResult<&[u8],_> { - Ok((_, stat)) => { - stat as u32 - } - _ => 0 as u32 + let stat : u32 = match be_u32(&r.prog_data) as IResult<&[u8],_> { + Ok((_, stat)) => stat, + _ => 0 }; nfs_status = stat; } @@ -122,7 +120,6 @@ impl NFSState { r.hdr.xid, xidmap.procedure, r.prog_data.len()); self.mark_response_tx_done(r.hdr.xid, r.reply_state, nfs_status, &resp_handle); - - 0 + return 0; } } diff --git a/rust/src/nfs/nfs3.rs b/rust/src/nfs/nfs3.rs index 3ca1c0b23c..8c2d2166ba 100644 --- a/rust/src/nfs/nfs3.rs +++ b/rust/src/nfs/nfs3.rs @@ -45,131 +45,92 @@ impl NFSState { self.process_request_record_lookup(r, &mut xidmap); } else if r.procedure == NFSPROC3_ACCESS { - match parse_nfs3_request_access(r.prog_data) { - Ok((_, ar)) => { - xidmap.file_handle = ar.handle.value.to_vec(); - self.xidmap_handle2name(&mut xidmap); - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_access(r.prog_data) { + xidmap.file_handle = rd.handle.value.to_vec(); + self.xidmap_handle2name(&mut xidmap); + } else { + self.set_event(NFSEvent::MalformedData); }; } else if r.procedure == NFSPROC3_GETATTR { - match parse_nfs3_request_getattr(r.prog_data) { - Ok((_, gar)) => { - xidmap.file_handle = gar.handle.value.to_vec(); - self.xidmap_handle2name(&mut xidmap); - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_getattr(r.prog_data) { + xidmap.file_handle = rd.handle.value.to_vec(); + self.xidmap_handle2name(&mut xidmap); + } else { + self.set_event(NFSEvent::MalformedData); }; } else if r.procedure == NFSPROC3_READDIRPLUS { - match parse_nfs3_request_readdirplus(r.prog_data) { - Ok((_, rdp)) => { - xidmap.file_handle = rdp.handle.value.to_vec(); - self.xidmap_handle2name(&mut xidmap); - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_readdirplus(r.prog_data) { + xidmap.file_handle = rd.handle.value.to_vec(); + self.xidmap_handle2name(&mut xidmap); + } else { + self.set_event(NFSEvent::MalformedData); }; } else if r.procedure == NFSPROC3_READ { - match parse_nfs3_request_read(r.prog_data) { - Ok((_, nfs3_read_record)) => { - xidmap.chunk_offset = nfs3_read_record.offset; - xidmap.file_handle = nfs3_read_record.handle.value.to_vec(); - self.xidmap_handle2name(&mut xidmap); - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_read(r.prog_data) { + xidmap.chunk_offset = rd.offset; + xidmap.file_handle = rd.handle.value.to_vec(); + self.xidmap_handle2name(&mut xidmap); + } else { + self.set_event(NFSEvent::MalformedData); }; } else if r.procedure == NFSPROC3_WRITE { - match parse_nfs3_request_write(r.prog_data) { - Ok((_, w)) => { - self.process_write_record(r, &w); - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_write(r.prog_data) { + self.process_write_record(r, &rd); + } else { + self.set_event(NFSEvent::MalformedData); } } else if r.procedure == NFSPROC3_CREATE { - match parse_nfs3_request_create(r.prog_data) { - Ok((_, nfs3_create_record)) => { - xidmap.file_handle = nfs3_create_record.handle.value.to_vec(); - xidmap.file_name = nfs3_create_record.name_vec; - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_create(r.prog_data) { + xidmap.file_handle = rd.handle.value.to_vec(); + xidmap.file_name = rd.name_vec; + } else { + self.set_event(NFSEvent::MalformedData); }; - } else if r.procedure == NFSPROC3_REMOVE { - match parse_nfs3_request_remove(r.prog_data) { - Ok((_, rr)) => { - xidmap.file_handle = rr.handle.value.to_vec(); - xidmap.file_name = rr.name_vec; - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_remove(r.prog_data) { + xidmap.file_handle = rd.handle.value.to_vec(); + xidmap.file_name = rd.name_vec; + } else { + self.set_event(NFSEvent::MalformedData); }; - } else if r.procedure == NFSPROC3_RENAME { - match parse_nfs3_request_rename(r.prog_data) { - Ok((_, rr)) => { - xidmap.file_handle = rr.from_handle.value.to_vec(); - xidmap.file_name = rr.from_name_vec; - aux_file_name = rr.to_name_vec; - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_rename(r.prog_data) { + xidmap.file_handle = rd.from_handle.value.to_vec(); + xidmap.file_name = rd.from_name_vec; + aux_file_name = rd.to_name_vec; + } else { + self.set_event(NFSEvent::MalformedData); }; } else if r.procedure == NFSPROC3_MKDIR { - match parse_nfs3_request_mkdir(r.prog_data) { - Ok((_, mr)) => { - xidmap.file_handle = mr.handle.value.to_vec(); - xidmap.file_name = mr.name_vec; - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_mkdir(r.prog_data) { + xidmap.file_handle = rd.handle.value.to_vec(); + xidmap.file_name = rd.name_vec; + } else { + self.set_event(NFSEvent::MalformedData); }; } else if r.procedure == NFSPROC3_RMDIR { - match parse_nfs3_request_rmdir(r.prog_data) { - Ok((_, rr)) => { - xidmap.file_handle = rr.handle.value.to_vec(); - xidmap.file_name = rr.name_vec; - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_request_rmdir(r.prog_data) { + xidmap.file_handle = rd.handle.value.to_vec(); + xidmap.file_name = rd.name_vec; + } else { + self.set_event(NFSEvent::MalformedData); }; } else if r.procedure == NFSPROC3_COMMIT { SCLogDebug!("COMMIT, closing shop"); - - match parse_nfs3_request_commit(r.prog_data) { - Ok((_, cr)) => { - let file_handle = cr.handle.value.to_vec(); - match self.get_file_tx_by_handle(&file_handle, STREAM_TOSERVER) { - Some((tx, files, flags)) => { - if let Some(NFSTransactionTypeData::FILE(ref mut tdf)) = tx.type_data { - tdf.chunk_count += 1; - tdf.file_additional_procs.push(NFSPROC3_COMMIT); - tdf.file_tracker.close(files, flags); - tdf.file_last_xid = r.hdr.xid; - tx.is_last = true; - tx.request_done = true; - } - }, - None => { }, + if let Ok((_, rd)) = parse_nfs3_request_commit(r.prog_data) { + let file_handle = rd.handle.value.to_vec(); + if let Some((tx, files, flags)) = self.get_file_tx_by_handle(&file_handle, STREAM_TOSERVER) { + if let Some(NFSTransactionTypeData::FILE(ref mut tdf)) = tx.type_data { + tdf.chunk_count += 1; + tdf.file_additional_procs.push(NFSPROC3_COMMIT); + tdf.file_tracker.close(files, flags); + tdf.file_last_xid = r.hdr.xid; + tx.is_last = true; + tx.request_done = true; } - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + } + } else { + self.set_event(NFSEvent::MalformedData); }; } @@ -216,19 +177,16 @@ impl NFSState { tx.nfs_version = r.progver as u16; tx.auth_type = r.creds_flavor; - match r.creds { - RpcRequestCreds::Unix(ref u) => { - tx.request_machine_name = u.machine_name_buf.to_vec(); - tx.request_uid = u.uid; - tx.request_gid = u.gid; - }, - _ => { }, + if let RpcRequestCreds::Unix(ref u) = r.creds { + tx.request_machine_name = u.machine_name_buf.to_vec(); + tx.request_uid = u.uid; + tx.request_gid = u.gid; } } } self.requestmap.insert(r.hdr.xid, xidmap); - 0 + return 0; } pub fn process_reply_record_v3<'b>(&mut self, r: &RpcReplyPacket<'b>, xidmap: &mut NFSRequestXidMap) -> u32 { @@ -236,97 +194,76 @@ impl NFSState { let mut resp_handle = Vec::new(); if xidmap.procedure == NFSPROC3_LOOKUP { - match parse_nfs3_response_lookup(r.prog_data) { - Ok((_, lookup)) => { - SCLogDebug!("LOOKUP: {:?}", lookup); - SCLogDebug!("RESPONSE LOOKUP file_name {:?}", xidmap.file_name); + if let Ok((_, rd)) = parse_nfs3_response_lookup(r.prog_data) { + SCLogDebug!("LOOKUP: {:?}", rd); + SCLogDebug!("RESPONSE LOOKUP file_name {:?}", xidmap.file_name); - nfs_status = lookup.status; + nfs_status = rd.status; - SCLogDebug!("LOOKUP handle {:?}", lookup.handle); - self.namemap.insert(lookup.handle.value.to_vec(), xidmap.file_name.to_vec()); - resp_handle = lookup.handle.value.to_vec(); - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + SCLogDebug!("LOOKUP handle {:?}", rd.handle); + self.namemap.insert(rd.handle.value.to_vec(), xidmap.file_name.to_vec()); + resp_handle = rd.handle.value.to_vec(); + } else { + self.set_event(NFSEvent::MalformedData); }; } else if xidmap.procedure == NFSPROC3_CREATE { - match parse_nfs3_response_create(r.prog_data) { - Ok((_, nfs3_create_record)) => { - SCLogDebug!("nfs3_create_record: {:?}", nfs3_create_record); - - SCLogDebug!("RESPONSE CREATE file_name {:?}", xidmap.file_name); - nfs_status = nfs3_create_record.status; + if let Ok((_, rd)) = parse_nfs3_response_create(r.prog_data) { + SCLogDebug!("nfs3_create_record: {:?}", rd); + SCLogDebug!("RESPONSE CREATE file_name {:?}", xidmap.file_name); + nfs_status = rd.status; - if let Some(h) = nfs3_create_record.handle { - SCLogDebug!("handle {:?}", h); - self.namemap.insert(h.value.to_vec(), xidmap.file_name.to_vec()); - resp_handle = h.value.to_vec(); - } - - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Some(h) = rd.handle { + SCLogDebug!("handle {:?}", h); + self.namemap.insert(h.value.to_vec(), xidmap.file_name.to_vec()); + resp_handle = h.value.to_vec(); + } + } else { + self.set_event(NFSEvent::MalformedData); }; } else if xidmap.procedure == NFSPROC3_READ { - match parse_nfs3_reply_read(r.prog_data) { - Ok((_, ref reply)) => { - self.process_read_record(r, reply, Some(&xidmap)); - nfs_status = reply.status; - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + if let Ok((_, rd)) = parse_nfs3_reply_read(r.prog_data) { + self.process_read_record(r, &rd, Some(&xidmap)); + nfs_status = rd.status; + } else { + self.set_event(NFSEvent::MalformedData); } } else if xidmap.procedure == NFSPROC3_READDIRPLUS { - match parse_nfs3_response_readdirplus(r.prog_data) { - Ok((_, ref reply)) => { - //SCLogDebug!("READDIRPLUS reply {:?}", reply); + if let Ok((_, rd)) = parse_nfs3_response_readdirplus(r.prog_data) { + nfs_status = rd.status; - nfs_status = reply.status; + // cut off final eof field + let d = if rd.data.len() >= 4 { + &rd.data[..rd.data.len()-4 as usize] + } else { + rd.data + }; - // cut off final eof field - let d = if reply.data.len() >= 4 { - &reply.data[..reply.data.len()-4 as usize] - } else { - reply.data - }; - - // store all handle/filename mappings - match many0_nfs3_response_readdirplus_entries(d) { - Ok((_, ref entries)) => { - SCLogDebug!("READDIRPLUS ENTRIES reply {:?}", entries); - for ce in entries { - SCLogDebug!("ce {:?}", ce); - if let Some(ref e) = ce.entry { - SCLogDebug!("e {:?}", e); - if let Some(ref h) = e.handle { - SCLogDebug!("h {:?}", h); - self.namemap.insert(h.value.to_vec(), - e.name_vec.to_vec()); - } - } + // store all handle/filename mappings + if let Ok((_, ref entries)) = many0_nfs3_response_readdirplus_entries(d) { + SCLogDebug!("READDIRPLUS ENTRIES reply {:?}", entries); + for ce in entries { + SCLogDebug!("ce {:?}", ce); + if let Some(ref e) = ce.entry { + SCLogDebug!("e {:?}", e); + if let Some(ref h) = e.handle { + SCLogDebug!("h {:?}", h); + self.namemap.insert(h.value.to_vec(), + e.name_vec.to_vec()); } - }, - _ => { - self.set_event(NFSEvent::MalformedData); - }, + } } - }, - _ => { + } else { self.set_event(NFSEvent::MalformedData); - }, + } + } else { + self.set_event(NFSEvent::MalformedData); } } // for all other record types only parse the status else { - let stat = match be_u32(&r.prog_data) as IResult<&[u8],_> { - Ok((_, stat)) => { - stat as u32 - } - _ => 0 as u32 + let stat : u32 = match be_u32(&r.prog_data) as IResult<&[u8],_> { + Ok((_, stat)) => stat, + _ => 0 }; nfs_status = stat; } @@ -337,6 +274,6 @@ impl NFSState { self.mark_response_tx_done(r.hdr.xid, r.reply_state, nfs_status, &resp_handle); } - 0 + return 0; } } diff --git a/rust/src/nfs/nfs4.rs b/rust/src/nfs/nfs4.rs index 16f06e9ac8..1d4f94ba89 100644 --- a/rust/src/nfs/nfs4.rs +++ b/rust/src/nfs/nfs4.rs @@ -54,15 +54,12 @@ impl NFSState { } let file_handle = fh.to_vec(); - let file_name = match self.namemap.get(fh) { - Some(n) => { - SCLogDebug!("WRITE name {:?}", n); - n.to_vec() - }, - None => { - SCLogDebug!("WRITE object {:?} not found", w.stateid.data); - Vec::new() - }, + let file_name = if let Some(name) = self.namemap.get(fh) { + SCLogDebug!("WRITE name {:?}", name); + name.to_vec() + } else { + SCLogDebug!("WRITE object {:?} not found", w.stateid.data); + Vec::new() }; let found = match self.get_file_tx_by_handle(&file_handle, STREAM_TOSERVER) { @@ -80,7 +77,7 @@ impl NFSState { } true }, - None => { false }, + None => false, }; if !found { let (tx, files, flags) = self.new_file_tx(&file_handle, &file_name, STREAM_TOSERVER); @@ -109,16 +106,13 @@ impl NFSState { SCLogDebug!("COMMIT, closing shop"); let file_handle = fh.to_vec(); - match self.get_file_tx_by_handle(&file_handle, STREAM_TOSERVER) { - Some((tx, files, flags)) => { - if let Some(NFSTransactionTypeData::FILE(ref mut tdf)) = tx.type_data { - tdf.file_tracker.close(files, flags); - tdf.file_last_xid = r.hdr.xid; - tx.is_last = true; - tx.request_done = true; - } + if let Some((tx, files, flags)) = self.get_file_tx_by_handle(&file_handle, STREAM_TOSERVER) { + if let Some(NFSTransactionTypeData::FILE(ref mut tdf)) = tx.type_data { + tdf.file_tracker.close(files, flags); + tdf.file_last_xid = r.hdr.xid; + tx.is_last = true; + tx.request_done = true; } - None => {}, } } @@ -286,7 +280,7 @@ impl NFSState { } self.requestmap.insert(r.hdr.xid, xidmap); - 0 + return 0; } fn compound_response<'b>(&mut self, r: &RpcReplyPacket<'b>, @@ -409,6 +403,6 @@ impl NFSState { }, }; } - 0 + return 0; } } -- 2.47.2