* [pve-devel] [PATCH 1/2] use anyhow for result/error
@ 2022-01-26 18:10 Thomas Lamprecht
2022-01-26 18:10 ` [pve-devel] [PATCH 2/2] switch from curl to ureq Thomas Lamprecht
2022-01-27 12:39 ` [pve-devel] [PATCH 1/2] use anyhow for result/error Dominik Csapak
0 siblings, 2 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2022-01-26 18:10 UTC (permalink / raw)
To: pve-devel
it's in out dependency chain anyway through proxmox-sys or -time and
it makes life a bit easier. FWIW, I got slightly pressured into this
by the future switch from curl to ureq, as the latter brings their
own Error type.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
Not a must, we can handle the ureq::Error otherwise, but as we already
depend on anyhow indirectly I figured to go for it...
Cargo.toml | 1 +
src/main.rs | 34 +++++++++++++++++-----------------
2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index 60c3ae0..25c4569 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -11,6 +11,7 @@ license = "AGPL-3"
exclude = [ "build", "debian" ]
[dependencies]
+anyhow = "1"
mio = { version = "0.7", features = [ "net", "os-ext" ] }
curl = "0.4"
clap = "2.33"
diff --git a/src/main.rs b/src/main.rs
index 7896a9c..0003834 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,12 +1,13 @@
use std::cmp::min;
use std::collections::HashMap;
use std::ffi::{OsStr, OsString};
-use std::io::{ErrorKind, Result, Write};
+use std::io::{ErrorKind, Write};
use std::os::unix::io::{AsRawFd, FromRawFd};
use std::os::unix::process::CommandExt;
use std::process::Command;
use std::time::{Duration, Instant};
+use anyhow::{bail, format_err, Result};
use clap::{App, AppSettings, Arg};
use curl::easy::Easy;
use mio::net::{TcpListener, TcpStream};
@@ -16,7 +17,6 @@ use mio::{Events, Interest, Poll, Token};
use proxmox_io::ByteBuffer;
use proxmox_sys::{
error::io_err_other,
- io_bail, io_format_err,
linux::pty::{make_controlling_terminal, PTY},
};
@@ -108,11 +108,11 @@ fn read_ticket_line(
match buf.read_from(stream) {
Ok(n) => {
if n == 0 {
- io_bail!("connection closed before authentication");
+ bail!("connection closed before authentication");
}
}
Err(err) if err.kind() == ErrorKind::WouldBlock => {}
- Err(err) => return Err(err),
+ Err(err) => return Err(err.into()),
}
if buf[..].contains(&b'\n') {
@@ -120,13 +120,13 @@ fn read_ticket_line(
}
if buf.is_full() {
- io_bail!("authentication data is incomplete: {:?}", &buf[..]);
+ bail!("authentication data is incomplete: {:?}", &buf[..]);
}
}
elapsed = now.elapsed();
if elapsed > timeout {
- io_bail!("timed out");
+ bail!("timed out");
}
}
@@ -140,7 +140,7 @@ fn read_ticket_line(
let (username, ticket) = line.split_at(pos);
Ok((username.into(), ticket[1..].into()))
}
- None => io_bail!("authentication data is invalid"),
+ None => bail!("authentication data is invalid"),
}
}
@@ -183,7 +183,7 @@ fn authenticate(
let response_code = curl.response_code()?;
if response_code != 200 {
- io_bail!("invalid authentication, code {}", response_code);
+ bail!("invalid authentication, code {}", response_code);
}
Ok(())
@@ -222,7 +222,7 @@ fn listen_and_accept(
elapsed = now.elapsed();
if elapsed > timeout {
- io_bail!("timed out");
+ bail!("timed out");
}
}
}
@@ -302,17 +302,17 @@ fn do_main() -> Result<()> {
let use_port_as_fd = matches.is_present("use-port-as-fd");
if use_port_as_fd && port > u16::MAX as u64 {
- return Err(io_format_err!("port too big"));
+ return Err(format_err!("port too big"));
} else if port > i32::MAX as u64 {
- return Err(io_format_err!("Invalid FD number"));
+ return Err(format_err!("Invalid FD number"));
}
let (mut tcp_handle, port) =
listen_and_accept("localhost", port, use_port_as_fd, Duration::new(10, 0))
- .map_err(|err| io_format_err!("failed waiting for client: {}", err))?;
+ .map_err(|err| format_err!("failed waiting for client: {}", err))?;
let (username, ticket) = read_ticket_line(&mut tcp_handle, &mut pty_buf, Duration::new(10, 0))
- .map_err(|err| io_format_err!("failed reading ticket: {}", err))?;
+ .map_err(|err| format_err!("failed reading ticket: {}", err))?;
let port = if use_port_as_fd { Some(port) } else { None };
authenticate(&username, &ticket, path, perm, authport, port)?;
tcp_handle.write_all(b"OK").expect("error writing response");
@@ -383,7 +383,7 @@ fn do_main() -> Result<()> {
}
Err(err) => {
if !finished {
- return Err(io_format_err!("error reading from tcp: {}", err));
+ return Err(format_err!("error reading from tcp: {}", err));
}
break;
}
@@ -403,7 +403,7 @@ fn do_main() -> Result<()> {
}
Err(err) => {
if !finished {
- return Err(io_format_err!("error reading from pty: {}", err));
+ return Err(format_err!("error reading from pty: {}", err));
}
break;
}
@@ -423,7 +423,7 @@ fn do_main() -> Result<()> {
}
Err(err) => {
if !finished {
- return Err(io_format_err!("error writing to tcp : {}", err));
+ return Err(format_err!("error writing to tcp : {}", err));
}
break;
}
@@ -447,7 +447,7 @@ fn do_main() -> Result<()> {
}
Err(err) => {
if !finished {
- return Err(io_format_err!("error writing to pty : {}", err));
+ return Err(format_err!("error writing to pty : {}", err));
}
break;
}
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] [PATCH 2/2] switch from curl to ureq
2022-01-26 18:10 [pve-devel] [PATCH 1/2] use anyhow for result/error Thomas Lamprecht
@ 2022-01-26 18:10 ` Thomas Lamprecht
2022-01-27 12:39 ` [pve-devel] [PATCH 1/2] use anyhow for result/error Dominik Csapak
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2022-01-26 18:10 UTC (permalink / raw)
To: pve-devel
Not only less code, we lose a whopping 29 libraries on linkage cruft:
--- ldd.before 2022-01-26 19:03:25.759426164 +0100
+++ ldd.after 2022-01-26 19:03:31.919529632 +0100
@@ -5,36 +5,7 @@ librt.so.1
libzstd.so.1
libc.so.6
/lib64/ld-linux-x86-64.so.2
-libcurl-gnutls.so.4
libgcc_s.so.1
libpthread.so.0
libm.so.6
libdl.so.2
-libnghttp2.so.14
-libidn2.so.0
-librtmp.so.1
-libssh2.so.1
-libpsl.so.5
-libnettle.so.8
-libgnutls.so.30
-libgssapi_krb5.so.2
-libldap_r-2.4.so.2
-liblber-2.4.so.2
-libbrotlidec.so.1
-libz.so.1
-libunistring.so.2
-libhogweed.so.6
-libgmp.so.10
-libgcrypt.so.20
-libp11-kit.so.0
-libtasn1.so.6
-libkrb5.so.3
-libk5crypto.so.3
-libcom_err.so.2
-libkrb5support.so.0
-libresolv.so.2
-libsasl2.so.2
-libbrotlicommon.so.1
-libgpg-error.so.0
-libffi.so.7
-libkeyutils.so.1
IOW.: curl is really nice for a CLI tool, but way to much overkill for simple
HTTP requests.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
IMO a no brainer, but I only did a quick "does it still work in general"
testing, I mean that *is* covering already quite a bit here ;-)
Cargo.toml | 2 +-
src/main.rs | 41 ++++++++++++++---------------------------
2 files changed, 15 insertions(+), 28 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index 25c4569..7f55edc 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -13,7 +13,7 @@ exclude = [ "build", "debian" ]
[dependencies]
anyhow = "1"
mio = { version = "0.7", features = [ "net", "os-ext" ] }
-curl = "0.4"
+ureq = { version = "2.4", default-features = false, features = [ "gzip", ] }
clap = "2.33"
proxmox-io = "1"
proxmox-sys = "0.2"
diff --git a/src/main.rs b/src/main.rs
index 0003834..29508e6 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -9,7 +9,6 @@ use std::time::{Duration, Instant};
use anyhow::{bail, format_err, Result};
use clap::{App, AppSettings, Arg};
-use curl::easy::Easy;
use mio::net::{TcpListener, TcpStream};
use mio::unix::SourceFd;
use mio::{Events, Interest, Poll, Token};
@@ -152,38 +151,26 @@ fn authenticate(
authport: u16,
port: Option<u16>,
) -> Result<()> {
- let mut curl = Easy::new();
- curl.url(&format!(
- "http://localhost:{}/api2/json/access/ticket",
- authport
- ))?;
-
- let username = curl.url_encode(username);
- let ticket = curl.url_encode(ticket);
- let path = curl.url_encode(path.as_bytes());
-
- let mut post_fields = Vec::with_capacity(5);
- post_fields.push(format!("username={}", username));
- post_fields.push(format!("password={}", ticket));
- post_fields.push(format!("path={}", path));
-
+ let mut post_fields: Vec<(&str, &str)> = Vec::with_capacity(5);
+ post_fields.append(& mut vec![
+ ("username", std::str::from_utf8(username)?),
+ ("password", std::str::from_utf8(ticket)?),
+ ("path", path),
+ ]);
if let Some(perm) = perm {
- let perm = curl.url_encode(perm.as_bytes());
- post_fields.push(format!("privs={}", perm));
+ post_fields.push(("privs", perm));
}
-
+ let port_str;
if let Some(port) = port {
- post_fields.push(format!("port={}", port));
+ port_str = port.to_string();
+ post_fields.push(("port", &port_str));
}
- curl.post_fields_copy(post_fields.join("&").as_bytes())?;
- curl.post(true)?;
- curl.perform()?;
+ let url = format!("http://localhost:{}/api2/json/access/ticket", authport);
+ let res = ureq::post(&url).send_form(&post_fields[..])?;
- let response_code = curl.response_code()?;
-
- if response_code != 200 {
- bail!("invalid authentication, code {}", response_code);
+ if res.status() != 200 {
+ bail!("invalid authentication, {} - {}", res.status(), res.status_text());
}
Ok(())
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH 1/2] use anyhow for result/error
2022-01-26 18:10 [pve-devel] [PATCH 1/2] use anyhow for result/error Thomas Lamprecht
2022-01-26 18:10 ` [pve-devel] [PATCH 2/2] switch from curl to ureq Thomas Lamprecht
@ 2022-01-27 12:39 ` Dominik Csapak
2022-01-28 8:21 ` [pve-devel] applied-series: " Thomas Lamprecht
1 sibling, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2022-01-27 12:39 UTC (permalink / raw)
To: Proxmox VE development discussion, Thomas Lamprecht
looks sane and works (tested on pve & pbs)
did not push them myself since the ldd diff made it
necessary that i edited the patch file before applying locally...
so, for both patches:
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] applied-series: [PATCH 1/2] use anyhow for result/error
2022-01-27 12:39 ` [pve-devel] [PATCH 1/2] use anyhow for result/error Dominik Csapak
@ 2022-01-28 8:21 ` Thomas Lamprecht
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2022-01-28 8:21 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
On 27.01.22 13:39, Dominik Csapak wrote:
> looks sane and works (tested on pve & pbs)
> did not push them myself since the ldd diff made it
> necessary that i edited the patch file before applying locally...
oops, yeah makes sense - I now added '> ' quoting in front of it to avoid
that.
> so, for both patches:
>
> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
> Tested-by: Dominik Csapak <d.csapak@proxmox.com>
Thanks! I added those for the first patch but I had to change 2/2 slightly,
as ureq returns an error-result for all responses > 400 and so I had to
match that to get it to print our error and strictly allow only 200 codes.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-28 8:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26 18:10 [pve-devel] [PATCH 1/2] use anyhow for result/error Thomas Lamprecht
2022-01-26 18:10 ` [pve-devel] [PATCH 2/2] switch from curl to ureq Thomas Lamprecht
2022-01-27 12:39 ` [pve-devel] [PATCH 1/2] use anyhow for result/error Dominik Csapak
2022-01-28 8:21 ` [pve-devel] applied-series: " Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox