-
Notifications
You must be signed in to change notification settings - Fork 73
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.
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
Make sure we handle failures in mediaproxied requests gracefully #505
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do a bit better :)
src/components/media-proxy.ts
Outdated
}, (getRes) => { | ||
try { | ||
const { statusCode } = res; | ||
res.setHeader('content-disposition', getRes.headers['content-disposition'] as string); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest something like:
if (getRes.headers["content-disposition"]) {
res.setHeader("content-disposition", getRes.headers["content-disposition"] as string);
}
if (getRes.headers["content-type"]) {
res.setHeader("content-type", getRes.headers["content-type"] as string);
}
if (getRes.headers["content-length"]) {
res.setHeader("content-length", getRes.headers["content-length"] as string);
}
which prevent the undefined values from throwing here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And now we can drop type casts too. The compiler was right all along :)
Fixes matrix-org/matrix-appservice-irc#1816 blowing up the whole bridge when the request fails for any reason.