diff --git a/CHANGELOG.md b/CHANGELOG.md index a6408f44..a85872b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `user.id` now uses `UserId` type, `ChatId` now represents only _chat id_, not channel username, all `chat_id` function parameters now accept `Recipient` [**BC**] +## 0.4.5 - 2022-04-03 + +### Fixed + +- Hide bot token in errors ([#200][200]) + +[200]: https://github.com/teloxide/teloxide-core/pull/200 + ## 0.4.4 - 2022-04-21 ### Added diff --git a/Cargo.toml b/Cargo.toml index b9ea7ec8..c672ab93 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "teloxide-core" description = "Core part of the `teloxide` library - telegram bot API client" -version = "0.4.4" +version = "0.4.5" edition = "2018" license = "MIT" diff --git a/src/bot/download.rs b/src/bot/download.rs index 5a98e488..4b07460b 100644 --- a/src/bot/download.rs +++ b/src/bot/download.rs @@ -41,6 +41,7 @@ impl<'w> Download<'w> for Bot { &self.token, path, ) + .map(|res| res.map_err(crate::errors::hide_token)) .boxed() } } diff --git a/src/errors.rs b/src/errors.rs index a0ac4567..be1adc40 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,19 +1,19 @@ use std::io; -use derive_more::From; use serde::Deserialize; use thiserror::Error; /// An error caused by downloading a file. -#[derive(Debug, Error, From)] +#[derive(Debug, Error)] pub enum DownloadError { /// A network error while downloading a file from Telegram. #[error("A network error: {0}")] + // NOTE: this variant must not be created by anything except the From impl Network(#[source] reqwest::Error), /// An I/O error while writing a file to destination. #[error("An I/O error: {0}")] - Io(#[source] std::io::Error), + Io(#[from] std::io::Error), } /// An error caused by sending a request to Telegram. @@ -35,6 +35,7 @@ pub enum RequestError { /// Network error while sending a request to Telegram. #[error("A network error: {0}")] + // NOTE: this variant must not be created by anything except the From impl Network(#[source] reqwest::Error), /// Error while parsing a response from Telegram. @@ -727,3 +728,65 @@ pub enum ApiError { #[error("Unknown error: {0:?}")] Unknown(String), } + +impl From for DownloadError { + fn from(error: reqwest::Error) -> Self { + DownloadError::Network(hide_token(error)) + } +} + +impl From for RequestError { + fn from(error: reqwest::Error) -> Self { + RequestError::Network(hide_token(error)) + } +} + +/// Replaces token in the url in the error with `token:redacted` string. +pub(crate) fn hide_token(mut error: reqwest::Error) -> reqwest::Error { + let url = match error.url_mut() { + Some(url) => url, + None => return error, + }; + + if let Some(mut segments) = url.path_segments() { + // Usually the url looks like "bot/..." or "file/bot/...". + let (beginning, segment) = match segments.next() { + Some("file") => ("file/", segments.next()), + segment => ("", segment), + }; + + if let Some(token) = segment.and_then(|s| s.strip_prefix("bot")) { + // make sure that what we are about to delete looks like a bot token + if let Some((id, secret)) = token.split_once(':') { + // The part before the : in the token is the id of the bot. + let id_character = |c: char| c.is_ascii_digit(); + + // The part after the : in the token is the secret. + // + // In all bot tokens we could find the secret is 35 characters long and is + // 0-9a-zA-Z_- only. + // + // It would be nice to research if TBA always has 35 character secrets or if it + // is just a coincidence. + const SECRET_LENGTH: usize = 35; + let secret_character = |c: char| c.is_ascii_alphanumeric() || c == '-' || c == '_'; + + if secret.len() >= SECRET_LENGTH + && id.chars().all(id_character) + && secret.chars().all(secret_character) + { + // found token, hide only the token + let without_token = + &url.path()[(beginning.len() + "/bot".len() + token.len())..]; + let redacted = format!("{beginning}token:redacted{without_token}"); + + url.set_path(&redacted); + return error; + } + } + } + } + + // couldn't find token in the url, hide the whole url + error.without_url() +} diff --git a/src/net/request.rs b/src/net/request.rs index 3730857e..1b2273cd 100644 --- a/src/net/request.rs +++ b/src/net/request.rs @@ -37,18 +37,14 @@ where let request = client .post(crate::net::method_url(api_url, token, method_name)) .multipart(params) - .build() - .map_err(RequestError::Network)?; + .build()?; // FIXME: uncomment this, when reqwest starts setting default timeout early // if let Some(timeout) = timeout_hint { // *request.timeout_mut().get_or_insert(Duration::ZERO) += timeout; // } - let response = client - .execute(request) - .await - .map_err(RequestError::Network)?; + let response = client.execute(request).await?; process_response(response).await } @@ -81,18 +77,14 @@ where .post(crate::net::method_url(api_url, token, method_name)) .header(CONTENT_TYPE, HeaderValue::from_static("application/json")) .body(params) - .build() - .map_err(RequestError::Network)?; + .build()?; // FIXME: uncomment this, when reqwest starts setting default timeout early // if let Some(timeout) = timeout_hint { // *request.timeout_mut().get_or_insert(Duration::ZERO) += timeout; // } - let response = client - .execute(request) - .await - .map_err(RequestError::Network)?; + let response = client.execute(request).await?; process_response(response).await } @@ -105,7 +97,7 @@ where tokio::time::sleep(DELAY_ON_SERVER_ERROR).await; } - let text = response.text().await.map_err(RequestError::Network)?; + let text = response.text().await?; serde_json::from_str::>(&text) .map_err(|source| RequestError::InvalidJson {