Make the feed URL editable, with a copy button
Entries and history are keyed by feed id, so changing a URL keeps them -- the point being that a feed URL can carry an auth token that gets rotated. Changing it clears the stored ETag/Last-Modified, which belong to the old URL and could otherwise produce a bogus 304. The copy button cannot use navigator.clipboard: that needs a secure context and this is served over plain HTTP on a LAN address. Falls back to execCommand. Invalid input now returns 400 rather than 500. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPyeapneuXrCdojsaiXGbe
This commit is contained in:
11
src/db.rs
11
src/db.rs
@@ -231,6 +231,17 @@ impl Db {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Forgets the cached ETag/Last-Modified. Those validators belong to the old URL, so
|
||||
/// keeping them across a URL change could produce a bogus 304 against the new one.
|
||||
pub fn clear_validators(&self, feed_id: &str) -> Result<()> {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
conn.execute(
|
||||
"UPDATE feeds SET etag = NULL, last_modified = NULL, last_checked = NULL WHERE id = ?1",
|
||||
[feed_id],
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn set_feed_error(&self, feed_id: &str, url: &str, msg: &str) -> Result<()> {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
conn.execute(
|
||||
|
||||
100
src/web.rs
100
src/web.rs
@@ -184,19 +184,56 @@ async fn feeds(State(state): State<WebState>) -> Result<Json<Vec<FeedRow>>, ApiE
|
||||
Ok(Json(out))
|
||||
}
|
||||
|
||||
/// Turns anyhow errors into a 500 with a readable body.
|
||||
pub struct ApiError(anyhow::Error);
|
||||
/// Validates a replacement feed URL: present, parseable, and not already subscribed under
|
||||
/// a different id. Returns the trimmed URL.
|
||||
fn check_url(
|
||||
url: &str,
|
||||
id: &str,
|
||||
feeds: &std::collections::BTreeMap<String, crate::config::Feed>,
|
||||
) -> Result<String, String> {
|
||||
let url = url.trim();
|
||||
if url.is_empty() {
|
||||
return Err("the feed URL cannot be empty".into());
|
||||
}
|
||||
match url::Url::parse(url) {
|
||||
Ok(u) if u.scheme() == "http" || u.scheme() == "https" => {}
|
||||
Ok(u) => return Err(format!("{:?} is not an http(s) URL", u.scheme())),
|
||||
Err(e) => return Err(format!("that is not a valid URL: {e}")),
|
||||
}
|
||||
if let Some((other, _)) = feeds.iter().find(|(k, f)| k.as_str() != id && f.url == url) {
|
||||
return Err(format!("{other:?} is already subscribed to that URL"));
|
||||
}
|
||||
Ok(url.to_owned())
|
||||
}
|
||||
|
||||
/// Turns errors into a response with a readable body. Bad input from the caller is a 400;
|
||||
/// anything else is a 500, because those are our fault and not the caller's.
|
||||
pub struct ApiError {
|
||||
error: anyhow::Error,
|
||||
status: StatusCode,
|
||||
}
|
||||
|
||||
impl<E: Into<anyhow::Error>> From<E> for ApiError {
|
||||
fn from(e: E) -> Self {
|
||||
Self(e.into())
|
||||
Self { error: e.into(), status: StatusCode::INTERNAL_SERVER_ERROR }
|
||||
}
|
||||
}
|
||||
|
||||
impl ApiError {
|
||||
fn bad_request(msg: impl Into<String>) -> Self {
|
||||
Self {
|
||||
error: anyhow::Error::msg(msg.into()),
|
||||
status: StatusCode::BAD_REQUEST,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl IntoResponse for ApiError {
|
||||
fn into_response(self) -> Response {
|
||||
tracing::warn!(error = ?self.0, "api error");
|
||||
(StatusCode::INTERNAL_SERVER_ERROR, format!("{:#}", self.0)).into_response()
|
||||
if self.status.is_server_error() {
|
||||
tracing::warn!(error = ?self.error, "api error");
|
||||
}
|
||||
(self.status, format!("{:#}", self.error)).into_response()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -213,6 +250,39 @@ mod tests {
|
||||
assert!(constant_time_eq("", ""));
|
||||
}
|
||||
|
||||
fn feed(url: &str) -> crate::config::Feed {
|
||||
crate::config::Feed {
|
||||
url: url.into(), folder: None, keywords: vec![], allow_explicit: false,
|
||||
auto_download: true, max_new_per_check: None, username: None,
|
||||
password: None, password_env: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn feed_urls_are_validated_before_being_saved() {
|
||||
let mut feeds = std::collections::BTreeMap::new();
|
||||
feeds.insert("a".to_string(), feed("https://a.example/rss"));
|
||||
feeds.insert("b".to_string(), feed("https://b.example/rss"));
|
||||
|
||||
// Rotating a token on your own feed is the point of making this editable.
|
||||
assert_eq!(
|
||||
check_url(" https://a.example/rss?auth=new ", "a", &feeds).unwrap(),
|
||||
"https://a.example/rss?auth=new",
|
||||
"whitespace is trimmed"
|
||||
);
|
||||
// Keeping your own URL unchanged is not a collision with yourself.
|
||||
assert!(check_url("https://a.example/rss", "a", &feeds).is_ok());
|
||||
|
||||
assert!(check_url("", "a", &feeds).is_err(), "empty");
|
||||
assert!(check_url(" ", "a", &feeds).is_err(), "whitespace only");
|
||||
assert!(check_url("not a url", "a", &feeds).is_err(), "unparseable");
|
||||
assert!(check_url("file:///etc/passwd", "a", &feeds).is_err(), "not http(s)");
|
||||
assert!(
|
||||
check_url("https://b.example/rss", "a", &feeds).is_err(),
|
||||
"another feed already has that URL"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generated_tokens_are_32_hex_chars_and_not_repeated() {
|
||||
let a = generate_token();
|
||||
@@ -292,6 +362,7 @@ async fn add_feed(
|
||||
/// Only the fields that are present are changed.
|
||||
#[derive(Deserialize)]
|
||||
struct FeedPatch {
|
||||
url: Option<String>,
|
||||
folder: Option<Option<String>>,
|
||||
keywords: Option<Vec<String>>,
|
||||
allow_explicit: Option<bool>,
|
||||
@@ -305,10 +376,22 @@ async fn patch_feed(
|
||||
Json(body): Json<FeedPatch>,
|
||||
) -> Result<StatusCode, ApiError> {
|
||||
let mut cfg = (*state.ctx.cfg()).clone();
|
||||
|
||||
let checked = match &body.url {
|
||||
Some(u) => Some(check_url(u, &id, &cfg.feeds).map_err(ApiError::bad_request)?),
|
||||
None => None,
|
||||
};
|
||||
|
||||
let feed = cfg
|
||||
.feeds
|
||||
.get_mut(&id)
|
||||
.ok_or_else(|| anyhow::anyhow!("no feed with id {id:?}"))?;
|
||||
.ok_or_else(|| ApiError::bad_request(format!("no feed with id {id:?}")))?;
|
||||
|
||||
let mut url_changed = false;
|
||||
if let Some(url) = checked {
|
||||
url_changed = url != feed.url;
|
||||
feed.url = url;
|
||||
}
|
||||
|
||||
if let Some(v) = body.folder {
|
||||
feed.folder = v.filter(|s| !s.trim().is_empty());
|
||||
@@ -327,6 +410,11 @@ async fn patch_feed(
|
||||
}
|
||||
cfg.save(&state.config_path)?;
|
||||
state.ctx.reload_cfg(&state.config_path)?;
|
||||
if url_changed {
|
||||
// Refreshing a rotated auth token is the common case; entries and download history
|
||||
// are keyed by feed id, so they survive the change.
|
||||
state.ctx.db.clear_validators(&id)?;
|
||||
}
|
||||
Ok(StatusCode::NO_CONTENT)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user