Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid reqwest panic on invalid URIs #557

Merged
merged 1 commit into from
Mar 22, 2022
Merged

Avoid reqwest panic on invalid URIs #557

merged 1 commit into from
Mar 22, 2022

Conversation

mre
Copy link
Member

@mre mre commented Mar 18, 2022

Temporary workaround for #539 until the issue gets fixed upstream.

@mre mre force-pushed the reqwest-panic-workaround branch from 9948d21 to 7f0689f Compare March 18, 2022 21:57
@lebensterben
Copy link
Member

I still wonder why reqwest cannot accept URLs like https://web.archive.org/web/20131015212820/http://www.deviantart.com/

This can be opened in browser.

@mre
Copy link
Member Author

mre commented Mar 18, 2022

If I change the test to your URL it works:

let client = ClientBuilder::builder().build().client().unwrap();
let res = client
    .check("https://web.archive.org/web/20131015212820/http://www.deviantart.com/")
    .await
    .unwrap();
assert!(res.status().is_success());

So it doesn't panic on that.
I will mention that in the original issue. (@dend fyi)

But the URL that I use in this PR (http://\") does panic.

@mre mre merged commit 45de5c7 into master Mar 22, 2022
@mre mre deleted the reqwest-panic-workaround branch March 22, 2022 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants