Skip to content
This repository has been archived by the owner on Oct 9, 2020. It is now read-only.

Tighten HttpAuthentication::with_fn bound from FnMut to Fn. #11

Merged
merged 2 commits into from
Jul 19, 2019
Merged

Tighten HttpAuthentication::with_fn bound from FnMut to Fn. #11

merged 2 commits into from
Jul 19, 2019

Conversation

qnighy
Copy link
Contributor

@qnighy qnighy commented Jul 16, 2019

Three constructor functions for HttpAuthentication only requires F: FnMut. However, the constructed instances are only usable when F: Fn. This is unfortunate for closures, since the passed closure will be inferred to be only FnMut when it only captures immutably.

use actix_web::{middleware, web, App, HttpServer};

use futures::future;

use actix_web_httpauth::middleware::HttpAuthentication;

fn main() -> std::io::Result<()> {
    HttpServer::new(|| {
        let auth = HttpAuthentication::basic(|req, _credentials| future::ok(req));
        App::new()
            .wrap(middleware::Logger::default())
            .wrap(auth) // Compile fails here
            .service(web::resource("/").to(|| "Test\r\n"))
    })
    .bind("127.0.0.1:8080")?
    .workers(1)
    .run()
}

Technically it's a breaking change, but as it's actually unusable without F: Fn, I'd think it's considered a bugfix. If this is unsuitable, I'll prepare another PR that introduces HttpAuthentication::with_fn2 etc.

@otavio
Copy link

otavio commented Jul 16, 2019

Agreed! I'd like it to go in as well :-)

@svartalf
Copy link
Collaborator

Hey, @qnighy, thank you for a contribution!

I'm a little busy today, so I'll review the changes tomorrow, but so far it looks okay to me.

@svartalf svartalf merged commit 4eeb1d9 into actix:master Jul 19, 2019
@svartalf
Copy link
Collaborator

Published as a 0.3.2 version, thanks once again!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants