FuelClient::new removes potentially critical path information
- Dominant language
- Rust
- Stars
- 56.8k
- Forks
- 2.9k
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 5
Description
`FuelClient::new` used the `FromStr` implementation of `FuelClient`. This implementation is curious:
```rust
impl FromStr for FuelClient {
type Err = anyhow::Error;
fn from_str(str: &str) -> Result {
let mut raw_url = str.to_string();
if !raw_url.starts_with("http") {
raw_url = format!("http://{raw_url}");
}
let mut url = reqwest::Url::parse(&raw_url)
.map_err(anyhow::Error::msg)
.with_context(|| format!("Invalid fuel-core URL: {str}"))?;
url.set_path("/v1/graphql");
...
}
}
```
Do you see it? `url.set_path` wipes out any path information on the url. Some node providers (quiknode) use the path to store auth information. Of course there is also the option to use headers for quiknode, but there is not an implementation for that here either.
I don't really understand why babysitting the url here is necessary, and it seems to me like this cleaning should just be removed and users be responsible for using correct url paths, however either way this should be rethought.
Contributor guide
Research direction
Start by locating FuelClient::new and the FromStr implementation shown in the issue, then trace how the parsed URL is used for GraphQL requests. Confirm the path is lost and review existing request-header handling; done should preserve supported provider authentication information, with tests covering the chosen URL and header behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, blockchain
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100