Skip to content

Commit 0eb2fda

Browse files
committed
fix: fetch_url: return err on non 2xx reponses
The main reason for this change is the app picker that Delta Chat clients use, which utilizes the `fetch_url` function. Sometimes we get an error from the server, but we have no way to figure out that it's an error, other than inspecting the body, which we don't (and shouldn't) do. This results in us attempting to send webxdc apps that are not even valid .zip files. Another, arguably even worse thing is that we also put the error responses to the cache, so it's not easy to recover from such an error. So, let's just return an error if the response code is not a successful response code.
1 parent 9d3450f commit 0eb2fda

File tree

4 files changed

+33
-0
lines changed

4 files changed

+33
-0
lines changed

deltachat-jsonrpc/src/api/types/message.rs

+1
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ impl MessageObject {
143143
let override_sender_name = message.get_override_sender_name();
144144

145145
let webxdc_info = if message.get_viewtype() == Viewtype::Webxdc {
146+
// WebxdcMessageInfo::get_for_message(context, msg_id).await.ok()
146147
Some(WebxdcMessageInfo::get_for_message(context, msg_id).await?)
147148
} else {
148149
None

src/chat.rs

+19
Original file line numberDiff line numberDiff line change
@@ -925,6 +925,24 @@ impl ChatId {
925925
}
926926
_ => {
927927
if msg.viewtype == Viewtype::File {
928+
// So, it's this guy? Do we have to validate the zip file?
929+
// But it's a little bruh, because it's sort of expensive.
930+
// We also can't avoid going from File to Webxdc
931+
// because we have a file picker to attach webxdc apps.
932+
//
933+
// Do we just handle the "invalid xdc"
934+
// when fetching / sending the draft / draft message?
935+
// How do we handle it though?
936+
// Do we keep Viewtype as Webxdc,
937+
// but return no `webxdcInfo`?
938+
// Well, if you think about it, it is possible to send
939+
// other `Viewtype` (e.g. video) even if the file
940+
// isn't a valid video file.
941+
// So, maybe we should do the same?
942+
// Just return `null` for `webxdc_info`.
943+
//
944+
// TODO also perhaps we need to document that when we use
945+
// Webxdc viewtype, it MUST be a valid file.
928946
if let Some((better_type, _)) = message::guess_msgtype_from_suffix(msg)
929947
// We do not do an automatic conversion to other viewtypes here so that
930948
// users can send images as "files" to preserve the original quality
@@ -2783,6 +2801,7 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
27832801
}
27842802
}
27852803
} else if msg.viewtype == Viewtype::Webxdc {
2804+
// Yeah, so the error occurs here when we do
27862805
context
27872806
.ensure_sendable_webxdc_file(&blob.to_abs_path())
27882807
.await?;

src/net/http.rs

+12
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,18 @@ async fn fetch_url(context: &Context, original_url: &str) -> Result<Response> {
261261
continue;
262262
}
263263

264+
if !response.status().is_success() {
265+
return Err(anyhow!(
266+
"The server returned a non-successful response code: {}{}",
267+
response.status().as_u16(),
268+
response
269+
.status()
270+
.canonical_reason()
271+
.map(|s| format!(" {s}"))
272+
.unwrap_or("".to_string())
273+
));
274+
}
275+
264276
let content_type = response
265277
.headers()
266278
.get("content-type")

src/webxdc.rs

+1
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ const STATUS_UPDATE_SIZE_MAX: usize = 100 << 10;
237237

238238
impl Context {
239239
/// check if a file is an acceptable webxdc for sending or receiving.
240+
// Wait, isn't this duplicated `ensure_sendable_webxdc_file`?
240241
pub(crate) async fn is_webxdc_file(&self, filename: &str, file: &[u8]) -> Result<bool> {
241242
if !filename.ends_with(WEBXDC_SUFFIX) {
242243
return Ok(false);

0 commit comments

Comments
 (0)