Skip to content

Commit

Permalink
refactor: improve HTTP client and documentation
Browse files Browse the repository at this point in the history
- Simplify HTTP client response handling
- Add explicit text/json content type methods
- Add HTTP status code verification
- Fix JWT logout on interrupt
- Prevent infinite loops during JWT renewal
- Remove credentials from documentation examples

This improves error handling robustness and security practices while
making the HTTP client interface cleaner and more explicit.
  • Loading branch information
roderickvd committed Jan 25, 2025
1 parent 20053ec commit e55769d
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 112 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ and [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/).
- [decoder] Always use accurate seeking mode for reliable position reporting
- [decoder] Fix logical error in `size_hint()` lower bound calculation
- [decoder] Remove `ExactSizeIterator` implementation as total samples can't be determined exactly
- [http] Simplified HTTP client response handling and content type management
- [http] Added status code checking in HTTP client responses
- [player] Improve seek logging with more detailed timestamps and progress information
- [remote] Improve network timeout handling and error messages

Expand Down
104 changes: 39 additions & 65 deletions src/gateway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
//!
//! # Example
//!
//! # Example
//!
//! ```rust
//! use pleezer::gateway::Gateway;
//!
Expand All @@ -48,11 +50,10 @@
//! let arl = gateway.oauth("[email protected]", "password").await?;
//! gateway.login_with_arl(&arl).await?;
//!
//! // Or use existing ARL
//! gateway.login_with_arl(&existing_arl).await?;
//!
//! // Session automatically renews
//! gateway.refresh().await?;
//! // Make authenticated requests
//! let songs = gateway.list_to_queue(&track_list).await?;
//! let recommendations = gateway.user_radio(user_id).await?;
//! let user_data = gateway.refresh().await?;
//! ```
use std::time::SystemTime;
Expand All @@ -62,8 +63,7 @@ use futures_util::TryFutureExt;
use md5::{Digest, Md5};
use reqwest::{
self,
header::{HeaderMap, HeaderValue, AUTHORIZATION, CONTENT_TYPE},
Body,
header::{HeaderMap, HeaderValue, AUTHORIZATION},
};
use serde::Deserialize;
use url::Url;
Expand Down Expand Up @@ -182,12 +182,6 @@ impl Gateway {
/// Returns access token on successful authentication.
const OAUTH_LOGIN_URL: &'static str = "https://connect.deezer.com/oauth/user_auth.php";

/// Content type for gateway requests.
///
/// Although the bodies of all gateway requests are JSON, the
/// `Content-Type` is not.
const PLAIN_TEXT_CONTENT: HeaderValue = HeaderValue::from_static("text/plain;charset=UTF-8");

/// Default empty JSON body for requests.
///
/// Used when a request requires a body but has no parameters.
Expand Down Expand Up @@ -377,6 +371,7 @@ impl Gateway {
/// Returns an error if:
/// * URL construction fails
/// * Network request fails
/// * HTTP status code is not successful (not 2xx)
/// * Response isn't valid JSON
/// * Response can't be parsed as type T
pub async fn request<T>(
Expand Down Expand Up @@ -404,19 +399,17 @@ impl Gateway {
self.client_id,
);
let url = url_str.parse::<reqwest::Url>()?;
let mut request = self.http_client.post(url, body);

let request_headers = request.headers_mut();
request_headers.try_insert(CONTENT_TYPE, Self::PLAIN_TEXT_CONTENT)?;

// Add any headers that were passed in.
// Although the bodies of all gateway requests are JSON, the
// `Content-Type` is not.
let mut request = self.http_client.text(url, body);
if let Some(headers) = headers {
request_headers.extend(headers);
// Add any headers that were passed in.
request.headers_mut().extend(headers);
}

let response = self.http_client.execute(request).await?;
let body = response.text().await?;

protocol::json(&body, T::METHOD)
}

Expand Down Expand Up @@ -725,7 +718,7 @@ impl Gateway {
// First get a session ID. The response can be ignored because the
// session ID is stored in the cookie store.
let request = self.http_client.get(Url::parse(Self::OAUTH_SID_URL)?, "");
let _ = self.http_client.execute(request).await?;
self.http_client.execute(request).await?;

// Then login and get an access token.
let query = Url::parse(&format!(
Expand All @@ -744,47 +737,6 @@ impl Gateway {
self.get_arl(&result.access_token).await
}

/// Performs JWT authentication request.
///
/// Internal helper for JWT operations that:
/// * Formats request URL with parameters
/// * Sends authenticated request
/// * Handles response and cookies
///
/// # Arguments
///
/// * `endpoint` - JWT endpoint path
/// * `body` - Request body content
///
/// # Errors
///
/// Returns error if:
/// * URL construction fails
/// * Network request fails
/// * Authentication fails
async fn jwt_auth<T>(&mut self, endpoint: &str, body: T) -> Result<()>
where
T: Into<Body>,
{
// `c` for cookie (headers), `p` for payload (body)
let query = Url::parse(&format!(
"{}{}?jo=p&rto=c&i=p",
Self::JWT_AUTH_URL,
endpoint
))?;

let request = self.http_client.post(query, body);
let response = self.http_client.execute(request).await?;

if !response.status().is_success() {
let message = response.text().await?;
return Err(Error::permission_denied(message));
}

// When successful, the `refresh-token` cookie is set within the HTTP client's cookie store.
Ok(())
}

/// Authenticates using JWT and ARL token.
///
/// Establishes a persistent session using:
Expand All @@ -804,12 +756,23 @@ impl Gateway {
/// * Network request fails
/// * Authentication fails
pub async fn login_with_arl(&mut self, arl: &Arl) -> Result<()> {
// `c` for cookie (headers), `p` for payload (body)
let query = Url::parse(&format!(
"{}{}?jo=p&rto=c&i=p",
Self::JWT_AUTH_URL,
Self::JWT_ENDPOINT_LOGIN
))?;

let auth = auth::Jwt {
arl: arl.to_string(),
account_id: self.user_token().await?.user_id.to_string(),
};
self.jwt_auth(Self::JWT_ENDPOINT_LOGIN, serde_json::to_string(&auth)?)
.await

let request = self.http_client.json(query, serde_json::to_string(&auth)?);
self.http_client.execute(request).await?;

// When successful, the `refresh-token` cookie is set within the HTTP client's cookie store.
Ok(())
}

/// Renews the current login session.
Expand All @@ -824,7 +787,18 @@ impl Gateway {
/// * Network request fails
/// * Token renewal fails
pub async fn renew_login(&mut self) -> Result<()> {
self.jwt_auth(Self::JWT_ENDPOINT_RENEW, "").await
// `c` for cookie (headers), `p` for payload (body)
let query = Url::parse(&format!(
"{}{}?jo=p&rto=c&i=c",
Self::JWT_AUTH_URL,
Self::JWT_ENDPOINT_RENEW
))?;

let request = self.http_client.json(query, Self::EMPTY_JSON_OBJECT);
self.http_client.execute(request).await?;

// When successful, the `refresh-token` cookie is set within the HTTP client's cookie store.
Ok(())
}

/// Logs out and invalidates the current session.
Expand Down
70 changes: 55 additions & 15 deletions src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,19 @@
//! // Or without session management for public endpoints
//! let client = Client::without_cookies(&config)?;
//!
//! // Make authenticated requests
//! let request = client.post(auth_url, credentials);
//! let response = client.execute(request).await?;
//! // Make API requests with appropriate content type
//! let request = client.json(api_url, track_data); // for JSON data
//! // or
//! let request = client.text(api_url, metadata); // for plain text data
//! let response = client.execute(request).await?; // checks HTTP status code
//!
//! // Cookies are automatically managed for session persistence
//! ```
use std::{future::Future, num::NonZeroU32, sync::Arc, time::Duration};
use std::{num::NonZeroU32, sync::Arc, time::Duration};

use futures_util::{FutureExt, TryFutureExt};
use governor::{DefaultDirectRateLimiter, Quota};
use http::header::CONTENT_TYPE;
use reqwest::{
self,
header::{HeaderValue, ACCEPT_LANGUAGE},
Expand All @@ -66,6 +68,7 @@ use crate::{config::Config, error::Result};
/// * Cookie-based session persistence
/// * Rate limiting for API quotas
/// * Consistent configuration
// TODO: implement builder pattern
pub struct Client {
/// Unlimited request client for special cases.
///
Expand Down Expand Up @@ -117,6 +120,16 @@ impl Client {
/// * Maintain responsive streaming
const READ_TIMEOUT: Duration = Duration::from_secs(2);

/// Content type for plain text requests.
///
/// Used by `text()` method to set Content-Type header to "text/plain;charset=UTF-8"
const CONTENT_TYPE_TEXT: HeaderValue = HeaderValue::from_static("text/plain;charset=UTF-8");

/// Content type for JSON requests.
///
/// Used by `json()` method to set Content-Type header to "application/json"
const CONTENT_TYPE_JSON: HeaderValue = HeaderValue::from_static("application/json");

/// Creates a new client with optional session management.
///
/// # Arguments
Expand Down Expand Up @@ -257,7 +270,7 @@ impl Client {
request
}

/// Builds a POST request.
/// Builds a POST request with plain text body.
///
/// Convenience method for `request()` with POST method.
///
Expand All @@ -266,12 +279,37 @@ impl Client {
/// * `url` - Request URL
/// * `body` - Request body content
#[inline]
pub fn post<U, T>(&self, url: U, body: T) -> reqwest::Request
pub fn text<U, T>(&self, url: U, body: T) -> reqwest::Request
where
U: Into<Url>,
T: Into<Body>,
{
self.request(Method::POST, url, body)
let mut request = self.request(Method::POST, url, body);
request
.headers_mut()
.insert(CONTENT_TYPE, Self::CONTENT_TYPE_TEXT);
request
}

/// Builds a POST request with JSON body.
///
/// Convenience method for `request()` with POST method.
///
/// # Arguments
///
/// * `url` - Request URL
/// * `body` - Request body content
#[inline]
pub fn json<U, T>(&self, url: U, body: T) -> reqwest::Request
where
U: Into<Url>,
T: Into<Body>,
{
let mut request = self.request(Method::POST, url, body);
request
.headers_mut()
.insert(CONTENT_TYPE, Self::CONTENT_TYPE_JSON);
request
}

/// Builds a GET request.
Expand All @@ -294,7 +332,8 @@ impl Client {
/// Executes a request with rate limiting.
///
/// Applies rate limiting before executing the request to
/// comply with API quotas.
/// comply with API quotas. Automatically verifies that the response
/// has a successful HTTP status code (2xx range).
///
/// # Arguments
///
Expand All @@ -305,14 +344,15 @@ impl Client {
/// Returns error if:
/// * Rate limiting fails
/// * Request execution fails
/// * Response status code is not successful (not 2xx)
/// * Network error occurs
pub fn execute(
&self,
request: reqwest::Request,
) -> impl Future<Output = Result<reqwest::Response>> + '_ {
pub async fn execute(&self, request: reqwest::Request) -> Result<reqwest::Response> {
// No need to await with jitter because the level of concurrency is low.
// TODO : use different rate limiter for each host.
let throttle = self.rate_limiter.until_ready();
throttle.then(|()| self.unlimited.execute(request).map_err(Into::into))
self.rate_limiter.until_ready().await;
match self.unlimited.execute(request).await {
Ok(response) => response.error_for_status().map_err(Into::into),
Err(e) => Err(e.into()),
}
}
}
Loading

0 comments on commit e55769d

Please sign in to comment.