Skip to content

Address ABS importer failures #1802

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.

Sign up for GitHub

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

Open
IgnisDa wants to merge 6 commits into
base: main
Choose a base branch
from
+149 −68
Open

Address ABS importer failures #1802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ad296de
feat: categorize Audiobookshelf import failures with explicit error t…
IgnisDa Jul 23, 2026
beb4d12
ci: Run CI
IgnisDa Jul 23, 2026
7c0bc0c
chore: migrate to new APIs
IgnisDa Jul 24, 2026
71c2f16
ci: Run CI
IgnisDa Jul 24, 2026
6d53cd1
chore: more defensive importer
IgnisDa Jul 24, 2026
e2984e3
ci: Run CI
IgnisDa Jul 24, 2026
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
14 changes: 14 additions & 0 deletions crates/models/external/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,20 @@ pub mod audiobookshelf {
pub struct ListResponse {
pub results: Vec<Item>,
}

#[derive(Debug, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct MediaProgress {
pub is_finished: bool,
pub episode_id: Option<String>,
pub library_item_id: Option<String>,
}

#[derive(Debug, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct MeResponse {
pub media_progress: Vec<MediaProgress>,
}
}

pub mod plex {
Expand Down
203 changes: 135 additions & 68 deletions crates/services/importer/audiobookshelf/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::{result::Result as StdResult, sync::Arc};
use std::{collections::HashSet, result::Result as StdResult, sync::Arc, time::Duration};

use anyhow::Result;
use anyhow::{Context, Result};
use application_utils::get_podcast_episode_number_by_name;
use common_utils::{get_http_client_with_tls_config, ryot_log};
use common_utils::{USER_AGENT_STR, ryot_log, sleep_for_n_seconds};
use data_encoding::BASE64;
use dependent_entity_utils::commit_metadata;
use dependent_models::{ImportCompletedItem, ImportOrExportMetadataItem, ImportResult};
Expand All @@ -18,8 +18,8 @@ use media_models::{
};
use openlibrary_provider::OpenlibraryService;
use reqwest::{
Client,
header::{AUTHORIZATION, HeaderValue},
Client, ClientBuilder,
header::{AUTHORIZATION, HeaderMap, HeaderValue, USER_AGENT},
};
use supporting_service::SupportingService;

Expand All @@ -28,6 +28,7 @@ struct ImportServices<'a> {
hardcover_service: &'a HardcoverService,
google_books_service: &'a GoogleBooksService,
open_library_service: &'a OpenlibraryService,
finished_podcast_episodes: &'a HashSet<(String, String)>,
}

pub async fn import(
Expand All @@ -40,19 +41,31 @@ pub async fn import(
let mut completed = vec![];
let mut failed = vec![];
let url = format!("{}/api", input.api_url);
let client = get_http_client_with_tls_config(
Some(vec![(
AUTHORIZATION,
HeaderValue::from_str(&format!("Bearer {}", input.api_key)).unwrap(),
)]),
let client = build_client(
&input.api_key,
input.allow_insecure_connections.unwrap_or(false),
);

let me = client
.get(format!("{url}/me"))
.send()
.await?
.json::<audiobookshelf_models::MeResponse>()
.await
.unwrap();
let finished_podcast_episodes = me
.media_progress
.into_iter()
.filter(|progress| progress.is_finished)
.filter_map(|progress| Some((progress.library_item_id?, progress.episode_id?)))
.collect::<HashSet<_>>();
Comment on lines +49 to +61

@coderabbitai coderabbitai Bot Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

/me response parsing panics on failure, unlike the rest of this PR.

.json::<MeResponse>().await.unwrap() panics the entire import job if the /me endpoint returns malformed JSON or an unexpected shape — a much larger blast radius than the per-item failures this PR otherwise carefully categorizes (network vs. status vs. parse errors) in get_item_details. Since import() already returns Result<ImportResult>, propagate this with ? and with_context instead.

🛠️ Proposed fix 
-    let me = client
-        .get(format!("{url}/me"))
-        .send()
-        .await?
-        .json::<audiobookshelf_models::MeResponse>()
-        .await
-        .unwrap();
+    let me = client
+        .get(format!("{url}/me"))
+        .send()
+        .await
+        .with_context(|| "Network error requesting Audiobookshelf /me endpoint")?
+        .json::<audiobookshelf_models::MeResponse>()
+        .await
+        .with_context(|| "Failed to parse JSON from Audiobookshelf /me response")?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let me = client
.get(format!("{url}/me"))
.send()
.await?
.json::<audiobookshelf_models::MeResponse>()
.await
.unwrap();
let finished_podcast_episodes = me
.media_progress
.into_iter()
.filter(|progress| progress.is_finished)
.filter_map(|progress| Some((progress.library_item_id?, progress.episode_id?)))
.collect::<HashSet<_>>();
let me = client
.get(format!("{url}/me"))
.send()
.await
.with_context(|| "Network error requesting Audiobookshelf /me endpoint")?
.json::<audiobookshelf_models::MeResponse>()
.await
.with_context(|| "Failed to parse JSON from Audiobookshelf /me response")?;
let finished_podcast_episodes = me
.media_progress
.into_iter()
.filter(|progress| progress.is_finished)
.filter_map(|progress| Some((progress.library_item_id?, progress.episode_id?)))
.collect::<HashSet<_>>();
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/services/importer/audiobookshelf/src/lib.rs` around lines 52 - 64, The
/me response parsing currently panics via unwrap instead of propagating errors.
In the import flow around the MeResponse deserialization, replace unwrap with
error propagation using ? and add contextual information with with_context,
preserving the existing Result-based handling.


let services = ImportServices {
ss,
hardcover_service,
google_books_service,
open_library_service,
finished_podcast_episodes: &finished_podcast_episodes,
};

let libraries_resp = client
Expand Down Expand Up @@ -80,7 +93,7 @@ pub async fn import(

let results: Vec<_> = stream::iter(finished_items.results.into_iter().enumerate())
.map(|(idx, item)| process_item(idx, item, len, &client, &url, &services))
.buffer_unordered(5)
.buffer_unordered(1)

@coderabbitai coderabbitai Bot Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🌐 Web query:

Is there a documented reason to fully serialize reqwest requests instead of using bounded concurrency with buffer_unordered?

💡 Result:

There is no documented requirement or technical advantage to fully serializing reqwest requests as a default practice. In fact, reqwest is designed for asynchronous operation, and using concurrency (such as with buffer_unordered) is the standard approach to maximize throughput and minimize latency by hiding network wait times [1][2][3]. However, there are specific, documented scenarios where serializing requests or limiting concurrency is necessary or preferred over unbound parallelism: 1. Server-Side Rate Limiting and Resource Constraints: Sending too many concurrent requests can trigger rate limits (429 Too Many Requests) or overwhelm the destination server, leading to dropped connections or timeouts [4]. 2. Client-Side Resource Exhaustion: Without bound, you risk exhausting local system resources, such as file descriptors or socket limits [4]. Connection pooling in reqwest is highly efficient, but unbounded concurrency can still cause connection churn, especially when interacting with many different hosts [5][3]. 3. Avoiding Thundering Herd Problems: In some complex architectures, spawning too many futures at once can create contention issues, where tasks wake up repeatedly but fail to progress, wasting CPU cycles [6]. 4. Order Sensitivity: If your application logic depends on the requests completing in a strict sequential order (e.g., streaming a process where step B requires the output of step A), buffer_unordered will break this sequence. In such cases, standard sequential iteration or buffer_ordered is required [7]. 5. Complex Data Dependencies: If requests are not independent—for example, if they require shared mutable state or perform operations that are not thread-safe (though reqwest::Client handles connection pooling internally and safely across threads)—serialization may be the simplest way to avoid complex synchronization overhead [1][5]. To balance performance with these constraints, it is common to use buffer_unordered(n) with a carefully chosen n (bounded concurrency) or a tokio::sync::Semaphore to manually control the number of active requests without resorting to full serialization [4][8]. The reqwest::Client should generally be created once and shared (cloned) across tasks to benefit from connection pooling, regardless of whether you choose to run requests concurrently or sequentially [5][3].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the relevant importer implementation and retry flow.
fd -a 'lib.rs' . | sed 's#^\./##' | rg 'importer/audiobookshelf/src/lib.rs|audiobookshelf/src/lib.rs' || true

echo '--- outline ---'
ast-grep outline crates/services/importer/audiobookshelf/src/lib.rs --view compact || true

echo '--- relevant lines ---'
cat -n crates/services/importer/audiobookshelf/src/lib.rs | sed -n '1,180p'

echo '--- search get_item_details and retry/backoff symbols ---'
rg -n "get_item_details|buffer_unordered|retry|backoff|try|Result|async|match" crates/services/importer/audiobookshelf/src/lib.rs

Repository: IgnisDa/ryot

Length of output: 9866

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- read slice around item processing continuation and get_item_details ---'
cat -n crates/services/importer/audiobookshelf/src/lib.rs | sed -n '150,320p'

echo '--- get_item_details usages and concurrent limits in audiobookshelf importer ---'
rg -n "get_item_details|buffer_unordered|buffer_max|Semaphore|semaphore|mutex|lock|finished_podcast|podcast" crates/services/importer/audiobookshelf/src/lib.rs crates -g '*.rs' | head -200

echo '--- determine whether get_item_details loop is exponential backoff and how many sleeps ---'
python3 - <<'PY'
from pathlib import Path
s = Path('crates/services/importer/audiobookshelf/src/lib.rs').read_text().splitlines()
for i, line in enumerate(s, 1):
    if i < 250 or i > 310:
        continue
    print(f"{i:04d}: {line}")
PY

Repository: IgnisDa/ryot

Length of output: 32688

Avoid serializing the entire importer with buffer_unordered(1).

Each podcast imports per-episode commits, while item-details fetching can retry forever with exponential sleeps (2s then 4s before the final attempt). Limit the risk with scoped synchronization instead of making every library item fetch and commit purely sequential; a bounded stream or per-podcast lock would preserve concurrency without exposing the same stale-during-retry failure window across the whole import pipeline.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/services/importer/audiobookshelf/src/lib.rs` at line 96, Replace the
global buffer_unordered(1) limit in the importer stream with scoped
synchronization, such as bounded concurrency or a per-podcast lock. Preserve
concurrency between independent library items while ensuring each podcast’s
episode imports and commits remain coordinated during item-details retries.

.collect()
.await;

Expand Down Expand Up @@ -120,73 +133,71 @@ async fn process_item(
_ => {
return Err(ImportFailedItem {
identifier: title,
lot: Some(MediaLot::Book),
step: ImportFailStep::InputTransformation,
error: Some("No Google Books ID found".to_string()),
..Default::default()
});
}
},
_ => {
return Err(ImportFailedItem {
identifier: title,
lot: Some(MediaLot::Book),
error: Some("No ISBN found".to_string()),
step: ImportFailStep::InputTransformation,
Comment on lines 143 to 147

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Reorder struct fields by ascending line length.

The changed ImportFailedItem initializers should consistently sort their fields by source-line length; notably, move shorter lot/step fields before the longer error expressions. As per coding guidelines, Rust struct fields must be ordered by ascending line length.

Also applies to: 143-148, 160-169, 184-189, 213-220

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/services/importer/audiobookshelf/src/lib.rs` around lines 130 - 134,
Reorder the fields in the affected ImportFailedItem initializers to follow
ascending source-line length, placing the shorter lot and step fields before the
longer error expressions. Apply this consistently to every initializer in the
importer, including the instances around the No ISBN found path and the
additional referenced locations, without changing values or behavior.

Source: Coding guidelines

..Default::default()
});
Comment on lines 134 to 148

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Include all source identifiers in failure messages.

The book failures at Lines 125 and 133 should include item.id. The episode database-commit failure at Line 186 should also include episode_id; otherwise failures cannot identify the exact Audiobookshelf record.

Also applies to: 184-189

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/services/importer/audiobookshelf/src/lib.rs` around lines 121 - 135,
Update the InputTransformation failure messages in the book import branches to
include both the existing reason and the Audiobookshelf record identifier
item.id. Also update the episode database-commit failure near the episode_id
handling to include episode_id in its error message, preserving the existing
failure steps and identifiers.

}
}
} else if let Some(asin) = metadata.asin.clone() {
(asin, MediaLot::AudioBook, MediaSource::Audible, None)
} else if let Some(itunes_id) = metadata.itunes_id.clone() {
let item_details = get_item_details(client, url, &item.id, None)
.await
.map_err(|e| ImportFailedItem {
identifier: title.clone(),
error: Some(e.to_string()),
step: ImportFailStep::ItemDetailsFromSource,
..Default::default()
})?;
let item_details =
get_item_details(client, url, &item.id)
.await
.map_err(|e| ImportFailedItem {
identifier: title.clone(),
error: Some(format!("item_id={}: {e:#}", item.id)),
lot: Some(MediaLot::Podcast),
step: ImportFailStep::ItemDetailsFromSource,
})?;
match item_details.media.and_then(|m| m.episodes) {
Some(episodes) => {
let lot = MediaLot::Podcast;
let source = MediaSource::Itunes;
let mut to_return = vec![];
for episode in episodes {
ryot_log!(debug, "Importing episode {:?}", episode.title);
let episode_details =
get_item_details(client, url, &item.id, Some(episode.id.unwrap()))
.await
.map_err(|e| ImportFailedItem {
identifier: title.clone(),
error: Some(e.to_string()),
step: ImportFailStep::ItemDetailsFromSource,
..Default::default()
})?;
if let Some(true) =
episode_details.user_media_progress.map(|u| u.is_finished)
let Some(episode_id) = episode.id.clone() else {
continue;
};
if !services
.finished_podcast_episodes
.contains(&(item.id.clone(), episode_id))
{
let (podcast, _) = commit_metadata(
PartialMetadataWithoutId {
lot,
source,
identifier: itunes_id.clone(),
..Default::default()
},
services.ss,
Some(true),
)
.await
.map_err(|e| ImportFailedItem {
identifier: title.clone(),
error: Some(e.to_string()),
step: ImportFailStep::ItemDetailsFromSource,
continue;
}
ryot_log!(debug, "Importing episode {:?}", episode.title);
let (podcast, _) = commit_metadata(
PartialMetadataWithoutId {
lot,
source,
identifier: itunes_id.clone(),
..Default::default()
})?;
if let Some(pe) = podcast.podcast_specifics.and_then(|p| {
get_podcast_episode_number_by_name(&p, &episode.title)
}) {
to_return.push(pe);
}
},
services.ss,
Some(true),
)
.await
.map_err(|e| ImportFailedItem {
identifier: title.clone(),
error: Some(format!("item_id={}: {e:#}", item.id)),
lot: Some(MediaLot::Podcast),
step: ImportFailStep::DatabaseCommit,
})?;
if let Some(pe) = podcast
.podcast_specifics
.and_then(|p| get_podcast_episode_number_by_name(&p, &episode.title))
{
to_return.push(pe);
}
}
(itunes_id, lot, source, Some(to_return))
Expand All @@ -196,7 +207,10 @@ async fn process_item(
identifier: title,
lot: Some(MediaLot::Podcast),
step: ImportFailStep::ItemDetailsFromSource,
error: Some("No episodes found for podcast".to_string()),
error: Some(format!(
"item_id={}: No episodes found for podcast",
item.id
)),
});
}
}
Expand All @@ -205,8 +219,11 @@ async fn process_item(
return Err(ImportFailedItem {
identifier: title,
step: ImportFailStep::InputTransformation,
error: Some("No ASIN, ISBN or iTunes ID found".to_string()),
..Default::default()
lot: None,
error: Some(format!(
"item_id={}: No ASIN, ISBN or iTunes ID found",
item.id
)),
});
};
let mut seen_history = vec![];
Expand Down Expand Up @@ -234,22 +251,72 @@ async fn process_item(
}))
}

const MAX_ITEM_DETAILS_ATTEMPTS: u32 = 3;

async fn get_item_details(
client: &Client,
url: &str,
id: &str,
episode: Option<String>,
) -> Result<audiobookshelf_models::Item> {
let mut query = serde_json::json!({ "expanded": "1", "include": "progress" });
if let Some(episode) = episode {
query["episode"] = serde_json::json!(episode);
let query = serde_json::json!({ "expanded": "1" });
let request_url = format!("{url}/items/{id}");
let mut last_error = None;
for attempt in 0..MAX_ITEM_DETAILS_ATTEMPTS {
match fetch_item_details(client, &request_url, &query).await {
Ok(item) => return Ok(item),
Err(error) => {
last_error = Some(error);
if attempt + 1 < MAX_ITEM_DETAILS_ATTEMPTS {
let sleep_time = u64::pow(2, attempt + 1);
ryot_log!(
debug,
"Attempt {}/{} failed for {}, retrying in {}s",
attempt + 1,
MAX_ITEM_DETAILS_ATTEMPTS,
request_url,
sleep_time
);
sleep_for_n_seconds(sleep_time).await;
}
}
}
}
let item = client
.get(format!("{url}/items/{id}"))
.query(&query)
Err(last_error.unwrap())
}
Comment on lines +254 to +285

@coderabbitai coderabbitai Bot Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Retry loop retries non-transient failures (parse errors, 4xx) the same as transient ones.

fetch_item_details errors from serde_json::from_str (malformed/unexpected JSON shape) and from non-success HTTP statuses (e.g. a deleted item returning 404) are indistinguishable from network errors here, so every permanent failure now burns the full 3-attempt exponential backoff (~2s + 4s ≈ 6s) before surfacing. Combined with buffer_unordered(1) (Line 96), this can meaningfully slow down imports for libraries containing any permanently-unreachable items. Consider only retrying on network errors or 5xx statuses, and failing fast on 4xx/parse errors.

[reliability_and_resilience]

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/services/importer/audiobookshelf/src/lib.rs` around lines 254 - 285,
Update get_item_details and fetch_item_details error handling so retries occur
only for network failures or HTTP 5xx responses; immediately return parse errors
and other non-retryable statuses such as 4xx responses. Preserve the existing
retry count, backoff, and successful item return behavior for retryable
failures.


async fn fetch_item_details(
client: &Client,
request_url: &str,
query: &serde_json::Value,
) -> Result<audiobookshelf_models::Item> {
let response = client
.get(request_url)
.query(query)
.send()
.await?
.json::<audiobookshelf_models::Item>()
.await?;
Ok(item)
.await
.with_context(|| format!("Network error requesting audiobookshelf API at {request_url}"))?;
let status = response.status();
let body = response.text().await.unwrap_or_default();
if !status.is_success() {
anyhow::bail!("Audiobookshelf returned HTTP {status} (url: {request_url}): {body}");
}
serde_json::from_str::<audiobookshelf_models::Item>(&body).with_context(|| {
format!("Failed to parse JSON from Audiobookshelf response for {request_url}")
})
}

// Longer timeout than the shared client since podcast item lookups can be slow.
fn build_client(bearer_token: &str, danger_accept_invalid_certs: bool) -> Client {
let mut headers = HeaderMap::new();
headers.insert(USER_AGENT, HeaderValue::from_static(USER_AGENT_STR));
headers.insert(
AUTHORIZATION,
HeaderValue::from_str(&format!("Bearer {bearer_token}")).unwrap(),
);
ClientBuilder::new()
.default_headers(headers)
.timeout(Duration::from_secs(60))
.danger_accept_invalid_certs(danger_accept_invalid_certs)
.build()
.unwrap()
}
Comment on lines +309 to 322

@coderabbitai coderabbitai Bot Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does reqwest HeaderValue::from_str reject leading/trailing whitespace or control characters in API tokens?

💡 Result:

Yes, the HeaderValue::from_str method in the http crate (which reqwest uses) rejects strings containing leading or trailing whitespace [1][2]. When you attempt to create a HeaderValue from a string that starts or ends with whitespace (such as a space or tab), the operation will return an error [1][2]. This design choice follows the HTTP specification (RFC 7230/9110), which dictates that header field values should not include leading or trailing whitespace, as such characters are meant to be excluded by parsers [1][2]. In addition to rejecting leading and trailing whitespace, HeaderValue::from_str validates that the input contains only visible ASCII characters (bytes 32–126, where 32 is a space) [3][4][5]. Although the horizontal tab (byte 9) is technically allowed in header values by the underlying logic, the specific restriction against leading/trailing whitespace prevents its use at the beginning or end of the string [1][5]. If you are dealing with API tokens that may contain accidental whitespace, you should trim the string (e.g., using.trim in Rust) before passing it to HeaderValue::from_str [2].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
git ls-files | rg 'crates/services/importer/audiobookshelf/src/lib\.rs|Cargo\.toml|Cargo.lock' || true

echo "== inspect relevant lines =="
cat -n crates/services/importer/audiobookshelf/src/lib.rs | sed -n '260,350p'

echo "== imports and surrounding config/token usage =="
sed -n '1,120p' crates/services/importer/audiobookshelf/src/lib.rs
rg -n "api_key|bearer_token|build_client|ImportResult|ImportFailedItem" crates/services/importer/audiobookshelf/src/lib.rs

echo "== reqwest/http dependency versions =="
rg -n 'reqwest|http \||http =|http ' Cargo.toml Cargo.lock

Repository: IgnisDa/ryot

Length of output: 15699

🌐 Web query:

http crate 1.4.0 HeaderValue::from_str leading whitespace leading/hard tabs v1 value parsing

💡 Result:

In the http crate, the HeaderValue::from_str method (and related constructors like from_bytes) strictly prohibits leading or trailing whitespace, as well as control characters (including tabs and other non-visible ASCII characters), by design [1][2][3]. 1. Strict Validation: The http crate enforces that a HeaderValue contains only visible ASCII characters (defined as the range 32-127 inclusive, excluding the DEL character 127) [1][4][5]. Because space (0x20) and horizontal tabs (0x09) are either outside this range or classified as non-visible/control characters in the context of this implementation, any input starting or ending with these characters, or containing them, will trigger a validation error [1][3]. 2. Motivation: This behavior is intentional. While HTTP specifications (such as RFC 7230/9110) note that parsers should ignore "Observed Whitespace" (OWS) around field values when extracting them from a raw byte stream, it is considered a requirement that a valid HeaderValue type itself should represent the "sanitized" or "trimmed" value [2][3]. Therefore, the library forces developers to ensure that the data provided to these constructors is already compliant and trimmed of leading/trailing whitespace [2][3]. 3. Historical Context: This behavior was formalized to prevent issues where invalid or ambiguous header values could be constructed [2][3]. Previous discussions and pull requests (e.g., #245, #256) confirmed that it is the responsibility of the library's constructors to validate that the provided string or byte slice adheres to these requirements [2][3]. If you are encountering errors when attempting to create a HeaderValue, ensure that the input string is trimmed of all leading and trailing whitespace and does not contain tabs or other non-printable ASCII characters before passing it to HeaderValue::from_str [1][6][3].

Citations:

🌐 Web query:

rust http 1.4.0 Source code HttpValue v1 header value visible ASCII 9 tab

💡 Result:

In the Rust http crate (version 1.4.0), the HeaderValue type supports the horizontal tab character (ASCII 9, \t) as a valid part of a header value [1][2]. Internally, the crate's validation logic uses a helper function called is_visible_ascii to determine which characters are permitted [1][2]. This function defines visible ASCII characters as those in the range 32–126 (inclusive) and explicitly includes the horizontal tab character (\t, ASCII 9) [1][2]. The code snippet below demonstrates this implementation logic: fn is_visible_ascii(b: u8) -> bool { b >= 32 && b < 127 || b == b'\t' } Consequently, methods that create a HeaderValue (such as from_static or from_str) will successfully accept strings containing horizontal tab characters [1][2]. This aligns with HTTP specifications that permit visible ASCII characters, spaces, and horizontal tabs within header field values [3].

Citations:

🌐 Web query:

http crate 1.4.0 HeaderValue::from_str trailing whitespace control character behavior leading/hard tab not visible ASCII

💡 Result:

In the http crate (including version 1.4.0), the HeaderValue::from_str method enforces strict validation rules on input strings [1][2]. It explicitly permits only visible ASCII characters, defined as the range 32–127 (inclusive) [1][3]. Regarding your specific concerns: 1. Leading and Trailing Whitespace: The http crate prohibits leading and trailing whitespace in HeaderValue objects [4]. This was implemented to align with HTTP specifications (RFC 7230), which expect parsers to ignore whitespace around header values rather than including them as part of the value itself [4]. Attempting to construct a HeaderValue with leading or trailing spaces (ASCII 32 or horizontal tab, ASCII 9) will result in an error [4][2]. 2. Control Characters and Tabs: The implementation restricts HeaderValue to visible ASCII characters (32–127) [1][3]. Because it explicitly excludes characters below 32, control characters—including horizontal tabs (ASCII 9)—are disallowed [1][3]. While some earlier discussions and GitHub issues noted potential nuances regarding how "visible ASCII" is interpreted versus broader HTTP spec requirements (which historically allowed HTAB), the current implementation maintains this strict restriction [1][5]. If you need to include opaque octets (bytes outside the 32–127 range, such as 128–255), you should use HeaderValue::from_bytes instead of from_str [1][2]. Unlike from_str, from_bytes provides a path to construct values with bytes that do not fit the visible ASCII requirement [1][3].

Citations:

Return a failed import item when the bearer token is not a valid HeaderValue.

HeaderValue::from_str(&format!("Bearer {bearer_token}")).unwrap() will panic for tokens containing leading/trailing whitespace or non-visible/control bytes. Since api_key is user-supplied, check the result here and add one failed item with the invalid-token error instead of crashing the import job. .build().unwrap() is comparatively low-risk because client build failures are TLS/layer initialization failures rather than config validation failures.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/services/importer/audiobookshelf/src/lib.rs` around lines 309 - 322,
Update build_client to handle HeaderValue::from_str failure without unwrapping:
propagate the invalid-token error to the importer so it returns one failed
import item instead of panicking. Adjust the function’s return flow and its
caller as needed while preserving normal client creation and existing
client-build behavior.

Loading