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

quick fix to gateway prober #153

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions ipfs-gateway-finder/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ async fn do_probing() -> Result<()> {
.long("gateway-list")
.value_name("URL")
.help("The URL of the JSON gateway list to use. Supported schemes are http, https, and file for local data")
.default_value("https://raw.githubusercontent.com/ipfs/public-gateway-checker/master/src/gateways.json")
.default_value("https://raw.githubusercontent.com/ipfs/public-gateway-checker/main/gateways.json")
.takes_value(true),
)
.arg(
Expand All @@ -141,11 +141,14 @@ async fn do_probing() -> Result<()> {

let monitor_api_address = matches.value_of("monitor_api_address").unwrap();
info!("using monitoring node API at {}", monitor_api_address);
let u = http::uri::Builder::new()
.scheme(http::uri::Scheme::HTTP)
.authority(monitor_api_address)
.path_and_query("/")
.build()

let monitor_api_address = if monitor_api_address.starts_with("http://") {
monitor_api_address.to_string()
} else {
format!("http://{}", monitor_api_address)
};

let u = monitor_api_address.parse::<http::Uri>()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this whole construction work? I don't remember if the path_and_query in the original was important... probably to strip any path components already present? But should be fine like this.

.context("invalid monitor_api_address")?;
let ipfs_client = IpfsClient::from_str(u.to_string().as_str())?;

Expand Down Expand Up @@ -178,7 +181,7 @@ async fn do_probing() -> Result<()> {
"getting list of gateways from {}...",
gateway_list_url.as_str()
);
let gateway_list: Vec<String> = match gateway_list_url.scheme() {
let mut gateway_list: Vec<String> = match gateway_list_url.scheme() {
"file" => {
let f = File::open(gateway_list_url.path()).context("unable to open file")?;
serde_json::from_reader(f).context("unable to deserialize gateway list")?
Expand All @@ -199,6 +202,18 @@ async fn do_probing() -> Result<()> {
)))
}
};

gateway_list = gateway_list
.into_iter()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can merge this with the part above by just appending everything from .map to the closing curly brace of the match expression. That avoids the mutable binding, which is always nice.
Alternatively, if that doesn't look nice, something like this also works:

let a: Vec<String> = {
    let mut a_mut = foo();
    a_mut.into_iter().do_stuff().collect()
}

This keeps the mutability inside the block.

.map(|gateway| {
if gateway.starts_with("http://") || gateway.starts_with("https://") {
gateway
} else {
format!("https://{}", gateway)
}
})
.collect();

info!("loaded {} gateways", gateway_list.len());
debug!("got gateways: {:?}", gateway_list);

Expand Down Expand Up @@ -554,8 +569,11 @@ async fn probe_gateway(
num_tries: u32,
timeout: Duration,
) -> Result<()> {
let mut url = Url::parse(gateway_url.replace(":hash", &cid).as_str())?;
let formatted_url = format!("{}/ipfs/{}", gateway_url, cid);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see, they removed the placeholders in the official list.
For backwards compatibility, could you check if the placeholder is there and only append this if it's not? I have a bunch of code relying on the old format...

let mut url = Url::parse(&formatted_url)?;
url.set_fragment(Some("x-ipfs-companion-no-redirect"));
debug!("constructed URL: {}", url);

let mut last_err = None;
// We use this to keep track of additional backoff timers.
// Specifically, if we get a response, but the wrong one, we assume something like an HTTP
Expand Down
Loading