Skip to content

Commit 3de12e1

Browse files
matt2xumattnenterprise
matt2xu
authored and
mattnenterprise
committed
Make code more ergonomic (#59)
* remove unnecessary allocations (calls to format! and String::from) * make write_str accepts S: AsRef<str> so we can pass a String without having to borrow it manually * make list_command accept Cow<'static, str> (better than AsRef because we can have both a &str or a String at calling site) * retr is generic over its result type (backward-compatible change) * rewrite retr in a more ergonomic way * rewrite simple_retr to use retr, remove simple_retr_ Possible backward-incompatible improvements, requiring major version changes: - make simple_retr return a Vec rather than a Cursor - change the signature of retr from Fn(&mut Read) to FnMut(R) where R: Read?
1 parent 20422f0 commit 3de12e1

File tree

1 file changed

+39
-75
lines changed

1 file changed

+39
-75
lines changed

‎src/ftp.rs

Lines changed: 39 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
//! FTP module.
2+
3+
use std::borrow::Cow;
14
use std::io::{Read, BufRead, BufReader, BufWriter, Cursor, Write, copy};
25
#[cfg(feature = "secure")]
36
use std::error::Error;
@@ -86,8 +89,7 @@ impl FtpStream {
8689
#[cfg(feature = "secure")]
8790
pub fn into_secure<T: IntoSsl + Clone>(mut self, ssl: T) -> Result<FtpStream> {
8891
// Ask the server to start securing data.
89-
let auth_command = String::from("AUTH TLS\r\n");
90-
try!(self.write_str(&auth_command));
92+
try!(self.write_str("AUTH TLS\r\n"));
9193
try!(self.read_response(status::AUTH_OK));
9294
let ssl_copy = try!(ssl.clone().into_ssl().map_err(|e| FtpError::SecureError(e.description().to_owned())));
9395
let stream = try!(SslStream::connect(ssl, self.reader.into_inner().into_tcp_stream())
@@ -97,12 +99,10 @@ impl FtpStream {
9799
ssl_cfg: Some(ssl_copy)
98100
};
99101
// Set protection buffer size
100-
let pbsz_command = format!("PBSZ 0\r\n");
101-
try!(secured_ftp_tream.write_str(&pbsz_command));
102+
try!(secured_ftp_tream.write_str("PBSZ 0\r\n"));
102103
try!(secured_ftp_tream.read_response(status::COMMAND_OK));
103104
// Change the level of data protectio to Private
104-
let prot_command = String::from("PROT P\r\n");
105-
try!(secured_ftp_tream.write_str(&prot_command));
105+
try!(secured_ftp_tream.write_str("PROT P\r\n"));
106106
try!(secured_ftp_tream.read_response(status::COMMAND_OK));
107107
Ok(secured_ftp_tream)
108108
}
@@ -130,8 +130,7 @@ impl FtpStream {
130130
#[cfg(feature = "secure")]
131131
pub fn into_insecure(mut self) -> Result<FtpStream> {
132132
// Ask the server to stop securing data
133-
let ccc_command = String::from("CCC\r\n");
134-
try!(self.write_str(&ccc_command));
133+
try!(self.write_str("CCC\r\n"));
135134
try!(self.read_response(status::COMMAND_OK));
136135
let plain_ftp_stream = FtpStream {
137136
reader: BufReader::new(DataStream::Tcp(self.reader.into_inner().into_tcp_stream())),
@@ -185,13 +184,11 @@ impl FtpStream {
185184

186185
/// Log in to the FTP server.
187186
pub fn login(&mut self, user: &str, password: &str) -> Result<()> {
188-
let user_command = format!("USER {}\r\n", user);
189-
try!(self.write_str(&user_command));
187+
try!(self.write_str(format!("USER {}\r\n", user)));
190188
self.read_response_in(&[status::LOGGED_IN, status::NEED_PASSWORD])
191189
.and_then(|Line(code, _)| {
192190
if code == status::NEED_PASSWORD {
193-
let pass_command = format!("PASS {}\r\n", password);
194-
try!(self.write_str(&pass_command));
191+
try!(self.write_str(format!("PASS {}\r\n", password)));
195192
try!(self.read_response(status::LOGGED_IN));
196193
}
197194
Ok(())
@@ -200,15 +197,13 @@ impl FtpStream {
200197

201198
/// Change the current directory to the path specified.
202199
pub fn cwd(&mut self, path: &str) -> Result<()> {
203-
let cwd_command = format!("CWD {}\r\n", path);
204-
try!(self.write_str(&cwd_command));
200+
try!(self.write_str(format!("CWD {}\r\n", path)));
205201
self.read_response(status::REQUESTED_FILE_ACTION_OK).map(|_| ())
206202
}
207203

208204
/// Move the current directory to the parent directory.
209205
pub fn cdup(&mut self) -> Result<()> {
210-
let cdup_command = format!("CDUP\r\n");
211-
try!(self.write_str(&cdup_command));
206+
try!(self.write_str("CDUP\r\n"));
212207
self.read_response(status::REQUESTED_FILE_ACTION_OK).map(|_| ())
213208
}
214209

@@ -231,15 +226,13 @@ impl FtpStream {
231226

232227
/// This does nothing. This is usually just used to keep the connection open.
233228
pub fn noop(&mut self) -> Result<()> {
234-
let noop_command = format!("NOOP\r\n");
235-
try!(self.write_str(&noop_command));
229+
try!(self.write_str("NOOP\r\n"));
236230
self.read_response(status::COMMAND_OK).map(|_| ())
237231
}
238232

239233
/// This creates a new directory on the server.
240234
pub fn mkdir(&mut self, pathname: &str) -> Result<()> {
241-
let mkdir_command = format!("MKD {}\r\n", pathname);
242-
try!(self.write_str(&mkdir_command));
235+
try!(self.write_str(format!("MKD {}\r\n", pathname)));
243236
self.read_response(status::PATH_CREATED).map(|_| ())
244237
}
245238

@@ -279,8 +272,7 @@ impl FtpStream {
279272

280273
/// Quits the current FTP session.
281274
pub fn quit(&mut self) -> Result<()> {
282-
let quit_command = format!("QUIT\r\n");
283-
try!(self.write_str(&quit_command));
275+
try!(self.write_str("QUIT\r\n"));
284276
self.read_response(status::CLOSING).map(|_| ())
285277
}
286278

@@ -296,12 +288,10 @@ impl FtpStream {
296288

297289
/// Renames the file from_name to to_name
298290
pub fn rename(&mut self, from_name: &str, to_name: &str) -> Result<()> {
299-
let rnfr_command = format!("RNFR {}\r\n", from_name);
300-
try!(self.write_str(&rnfr_command));
291+
try!(self.write_str(format!("RNFR {}\r\n", from_name)));
301292
self.read_response(status::REQUEST_FILE_PENDING)
302293
.and_then(|_| {
303-
let rnto_command = format!("RNTO {}\r\n", to_name);
304-
try!(self.write_str(&rnto_command));
294+
try!(self.write_str(format!("RNTO {}\r\n", to_name)));
305295
self.read_response(status::REQUESTED_FILE_ACTION_OK).map(|_| ())
306296
})
307297
}
@@ -326,32 +316,15 @@ impl FtpStream {
326316
/// }).is_ok());
327317
/// # assert!(conn.rm("retr.txt").is_ok());
328318
/// ```
329-
pub fn retr<F>(&mut self, filename: &str, reader: F) -> Result<()>
330-
where F: Fn(&mut Read) -> Result<()> {
319+
pub fn retr<F, T>(&mut self, filename: &str, reader: F) -> Result<T>
320+
where F: Fn(&mut Read) -> Result<T> {
331321
let retr_command = format!("RETR {}\r\n", filename);
332-
let mut data_stream = BufReader::new(try!(self.data_command(&retr_command)));
333-
self.read_response_in(&[status::ABOUT_TO_SEND, status::ALREADY_OPEN])
334-
.and_then(|_| {
335-
let result = reader(&mut data_stream);
336-
drop(data_stream);
337-
try!(self.read_response(status::CLOSING_DATA_CONNECTION));
338-
result
339-
})
340-
}
341-
342-
fn simple_retr_(&mut self, file_name: &str) -> Result<Cursor<Vec<u8>>> {
343-
let mut data_stream = try!(self.get(file_name));
344-
let buffer: &mut Vec<u8> = &mut Vec::new();
345-
loop {
346-
let mut buf = [0; 256];
347-
let len = try!(data_stream.read(&mut buf).map_err(|read_err| FtpError::ConnectionError(read_err)));
348-
if len == 0 {
349-
break;
350-
}
351-
try!(buffer.write(&buf[0..len]).map_err(|write_err| FtpError::ConnectionError(write_err)));
352-
}
353-
drop(data_stream);
354-
Ok(Cursor::new(buffer.clone()))
322+
{
323+
let mut data_stream = BufReader::new(try!(self.data_command(&retr_command)));
324+
self.read_response_in(&[status::ABOUT_TO_SEND, status::ALREADY_OPEN])
325+
.and_then(|_| reader(&mut data_stream))
326+
}.and_then(|res|
327+
self.read_response(status::CLOSING_DATA_CONNECTION).map(|_| res))
355328
}
356329

357330
/// Simple way to retr a file from the server. This stores the file in memory.
@@ -370,21 +343,21 @@ impl FtpStream {
370343
/// # assert!(conn.rm("simple_retr.txt").is_ok());
371344
/// ```
372345
pub fn simple_retr(&mut self, file_name: &str) -> Result<Cursor<Vec<u8>>> {
373-
let r = try!(self.simple_retr_(file_name));
374-
self.read_response(status::CLOSING_DATA_CONNECTION).map(|_| r)
346+
self.retr(file_name, |reader| {
347+
let mut buffer = Vec::new();
348+
reader.read_to_end(&mut buffer).map(|_| buffer).map_err(|read_err| FtpError::ConnectionError(read_err))
349+
}).map(|buffer| Cursor::new(buffer))
375350
}
376351

377352
/// Removes the remote pathname from the server.
378353
pub fn rmdir(&mut self, pathname: &str) -> Result<()> {
379-
let rmd_command = format!("RMD {}\r\n", pathname);
380-
try!(self.write_str(&rmd_command));
354+
try!(self.write_str(format!("RMD {}\r\n", pathname)));
381355
self.read_response(status::REQUESTED_FILE_ACTION_OK).map(|_| ())
382356
}
383357

384358
/// Remove the remote file from the server.
385359
pub fn rm(&mut self, filename: &str) -> Result<()> {
386-
let rm_command = format!("DELE {}\r\n", filename);
387-
try!(self.write_str(&rm_command));
360+
try!(self.write_str(format!("DELE {}\r\n", filename)));
388361
self.read_response(status::REQUESTED_FILE_ACTION_OK).map(|_| ())
389362
}
390363

@@ -404,7 +377,7 @@ impl FtpStream {
404377
}
405378

406379
/// Execute a command which returns list of strings in a separate stream
407-
fn list_command(&mut self, cmd: String, open_code: u32, close_code: u32) -> Result<Vec<String>> {
380+
fn list_command(&mut self, cmd: Cow<'static, str>, open_code: u32, close_code: u32) -> Result<Vec<String>> {
408381
let mut lines: Vec<String> = Vec::new();
409382
{
410383
let mut data_stream = BufReader::new(try!(self.data_command(&cmd)));
@@ -427,10 +400,7 @@ impl FtpStream {
427400
/// If `pathname` is omited then the list of files in the current directory will be
428401
/// returned otherwise it will the list of files on `pathname`.
429402
pub fn list(&mut self, pathname: Option<&str>) -> Result<Vec<String>> {
430-
let command = match pathname {
431-
Some(path) => format!("LIST {}\r\n", path),
432-
None => String::from("LIST\r\n"),
433-
};
403+
let command = pathname.map_or("LIST\r\n".into(), |path| format!("LIST {}\r\n", path).into());
434404

435405
self.list_command(command, status::ABOUT_TO_SEND, status::CLOSING_DATA_CONNECTION)
436406
}
@@ -439,19 +409,15 @@ impl FtpStream {
439409
/// If `pathname` is omited then the list of files in the current directory will be
440410
/// returned otherwise it will the list of files on `pathname`.
441411
pub fn nlst(&mut self, pathname: Option<&str>) -> Result<Vec<String>> {
442-
let command = match pathname {
443-
Some(path) => format!("NLST {}\r\n", path),
444-
None => String::from("NLST\r\n"),
445-
};
412+
let command = pathname.map_or("NLST\r\n".into(), |path| format!("NLST {}\r\n", path).into());
446413

447414
self.list_command(command, status::ABOUT_TO_SEND, status::CLOSING_DATA_CONNECTION)
448415
}
449416

450417
/// Retrieves the modification time of the file at `pathname` if it exists.
451418
/// In case the file does not exist `None` is returned.
452419
pub fn mdtm(&mut self, pathname: &str) -> Result<Option<DateTime<UTC>>> {
453-
let mdtm_command = format!("MDTM {}\r\n", pathname);
454-
try!(self.write_str(&mdtm_command));
420+
try!(self.write_str(format!("MDTM {}\r\n", pathname)));
455421
let Line(_, content) = try!(self.read_response(status::FILE));
456422

457423
match MDTM_RE.captures(&content) {
@@ -475,8 +441,7 @@ impl FtpStream {
475441
/// Retrieves the size of the file in bytes at `pathname` if it exists.
476442
/// In case the file does not exist `None` is returned.
477443
pub fn size(&mut self, pathname: &str) -> Result<Option<usize>> {
478-
let size_command = format!("SIZE {}\r\n", pathname);
479-
try!(self.write_str(&size_command));
444+
try!(self.write_str(format!("SIZE {}\r\n", pathname)));
480445
let Line(_, content) = try!(self.read_response(status::FILE));
481446

482447
match SIZE_RE.captures(&content) {
@@ -485,14 +450,13 @@ impl FtpStream {
485450
}
486451
}
487452

488-
fn write_str(&mut self, s: &str) -> Result<()> {
489-
let stream = self.reader.get_mut();
490-
453+
fn write_str<S: AsRef<str>>(&mut self, command: S) -> Result<()> {
491454
if cfg!(feature = "debug_print") {
492-
print!("CMD {}", s);
455+
print!("CMD {}", command.as_ref());
493456
}
494457

495-
stream.write_fmt(format_args!("{}", s))
458+
let stream = self.reader.get_mut();
459+
stream.write_all(command.as_ref().as_bytes())
496460
.map_err(|send_err| FtpError::ConnectionError(send_err))
497461
}
498462

0 commit comments

Comments
 (0)