Cryptographically verifiable, distributed dependency reviews
Add the last reviewed version to Cargo.toml / [dependencies]:
ureq = "0.11.1"
Please, use mobile in landscape.
Filter reviews clicking on the numbers in the summary.
Full column names in tooltip hints: rating Negative, rating Neutral, rating Positive, rating Strong, thoroughness, understanding, reviews count.
© bestia.dev 2023, MIT License, Version: 2023.608.1636
Open source repository for this web app: https://github.com/bestia-dev/cargo_crev_web/
Header::new
is unsound? Thisunsafe
there seem unneccessary in the first place. There's not much performance to gain here.Header: value
- there can be more spaces preceeding the value.Header::from_str
could take aHeaderName: Value
"The field value MAY be preceded by any amount of LWS, though a single SP is preferred. " (https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2)Probably can be done faster by just comparing slice with
eq_ignore_ascii_case
, instead of allocating a lowercase copy.src/lib.rs
: Tests doing http calls to external network can fail on offline machines, are a potential privacy problem etc.PoolKey::new
- failing to get a port should probably be an error, since that means te scheme was neither http nor https, so why are we even handling it?Unit
indeed is a so-so name. If the comment is "unit of work" then it probably should beUnitOfWork
.Ouch. When I see mutable raw pointers, I already know that I will not be using this code as is. :D . From what I can see later, it seems this pointer is used just for the drop implementation? In that case, just use
Option<Stream>
orOption<Box<Stream>>
.Option<Box<T>>
even compiles down to the same data/code as nullable pointer.I fail to see the point of
Request::build
...Request::query
andquery_str
seems silent about the matter of escaping, and I wonder if it will work correctly at all.Request::timeout
... Deadlines are better than timeouts, and are not harder to implement.index: (usize, usize), // index into status_line where we split: HTTP/1.1 200 OK
I see no reason, why these two would be touple, instead of being separate and named appropriatly.
Response::new
works by ... parsing? I don't know how I feel about that. Seems wasteful.I don't know how I feel about this. Seems like a bad idea. :D . It will lead to confusion during debugging eg. by people who don't know about this "feature" (eg. DevOps that will be reading logs of software that is using this library). They will see "error: 535", and wonder how the hell this code happened.
let mut yolo = YoloRead {
does not build confidence. :DOh. Here is another
*mut Stream
. I don't really get why it is neccessary.I don't think this clone is neccessary.
Generally - negative review, since there's some
unsafe
code that I don't think is neccessary, and I haveno confidence that it is actually correct (quite the opposite... I suspect some stuff is wrong with it).
There were some other minor problems, potentially bugs, a lot of casual needless cloning and
stuff that looks like plain inefficiencies, and generally this crate at its current state does not look
like something I'd recommend for any serious production use. The goal seems good but it seem not there yet.
I think crates like this need either a lot of usage and pair of eyes and developers to iron out all the details,
or some extensive test suite.