fix(attachments): map MIME from extension + don't treat 403 as auth
Two bugs found by reading the log carefully — both shipped in the
initial attachments feature:
1. MailItemExtractor sent every attachment with ContentType = "" and
UploadAttachmentAsync defaulted that to "application/octet-stream".
EspoCRM's attachment upload whitelist (config
attachmentUpload.fileAcceptOptions) does not include octet-stream,
so the server replied:
403 (X-Status-Reason=Not allowed file type.)
Add a GuessMimeType(fileName) helper to MailItemExtractor that maps
common extensions (pdf, doc/docx, xls/xlsx, ppt/pptx, txt, csv,
images, archives, email, audio/video) to canonical MIMEs. The
octet-stream is now only used for truly unknown extensions.
2. 403 was being conflated with 401 in THREE places — EnsureSuccessAsync,
the inline UploadAttachmentAsync handler, and the inline
CreateDocumentAsync handler. Result: a single "Not allowed file
type" rejection threw EspoCrmAuthorizationException, which
AttachmentSaveService treats as a batch-level auth failure and
stops processing remaining files. (Same bug would have killed
FilingService and MatchingService for any non-auth 403 — e.g. an
ACL miss on one record stopping a multi-mail file.)
Only 401 is unambiguously an auth issue. 403 means "you ARE who
you say you are, but this specific operation isn't allowed" —
content rejected / ACL miss / role missing. Treat 403 as a normal
HTTP error: log the body + X-Status-Reason and surface the message
to the user without aborting the batch.
Also include the X-Status-Reason header in the generic
EspoCrmHttpException message everywhere, so any future "fileSizeTooBig"
or similar shows up in the user-visible error and not just the log.
Tests: 39 passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -238,7 +238,8 @@ namespace MarcusLaw.OutlookAddin.Core.Services
|
||||
_logger.Warning(
|
||||
"UploadAttachment failed: {Status} (X-Status-Reason={Reason}) body={Body}",
|
||||
response.StatusCode, reason, string.IsNullOrEmpty(errBody) ? "(empty)" : Truncate(errBody, 500));
|
||||
if (response.StatusCode == HttpStatusCode.Unauthorized || response.StatusCode == HttpStatusCode.Forbidden)
|
||||
// 401 only = auth. 403 here is content/permission, not creds.
|
||||
if (response.StatusCode == HttpStatusCode.Unauthorized)
|
||||
{
|
||||
throw new EspoCrmAuthorizationException(
|
||||
$"EspoCRM rejected credentials ({(int)response.StatusCode} {response.StatusCode}). Re-enter API key.");
|
||||
@@ -294,7 +295,7 @@ namespace MarcusLaw.OutlookAddin.Core.Services
|
||||
_logger.Warning(
|
||||
"CreateDocument failed: {Status} (X-Status-Reason={Reason}) body={Body}",
|
||||
response.StatusCode, reason, string.IsNullOrEmpty(errBody) ? "(empty)" : Truncate(errBody, 500));
|
||||
if (response.StatusCode == HttpStatusCode.Unauthorized || response.StatusCode == HttpStatusCode.Forbidden)
|
||||
if (response.StatusCode == HttpStatusCode.Unauthorized)
|
||||
{
|
||||
throw new EspoCrmAuthorizationException(
|
||||
$"EspoCRM rejected credentials ({(int)response.StatusCode} {response.StatusCode}). Re-enter API key.");
|
||||
@@ -541,14 +542,20 @@ namespace MarcusLaw.OutlookAddin.Core.Services
|
||||
if (response.IsSuccessStatusCode) return;
|
||||
|
||||
var body = await ReadBodyAsync(response).ConfigureAwait(false);
|
||||
if (response.StatusCode == HttpStatusCode.Unauthorized || response.StatusCode == HttpStatusCode.Forbidden)
|
||||
// Only 401 is unambiguously an authentication problem. 403 in
|
||||
// EspoCRM is "you are who you say you are, but THIS operation
|
||||
// isn't allowed" — content type rejected, ACL miss, required
|
||||
// role missing, etc. Conflating them aborts whole batches.
|
||||
if (response.StatusCode == HttpStatusCode.Unauthorized)
|
||||
{
|
||||
throw new EspoCrmAuthorizationException(
|
||||
$"EspoCRM rejected credentials ({(int)response.StatusCode} {response.StatusCode}). Re-enter API key.");
|
||||
}
|
||||
|
||||
var reason = response.Headers.TryGetValues("X-Status-Reason", out var v)
|
||||
? " (reason: " + string.Join(",", v) + ")" : string.Empty;
|
||||
throw new EspoCrmHttpException(response.StatusCode, body,
|
||||
$"EspoCRM returned {(int)response.StatusCode} {response.StatusCode}: {Truncate(body, 500)}");
|
||||
$"EspoCRM returned {(int)response.StatusCode} {response.StatusCode}{reason}: {Truncate(body, 500)}");
|
||||
}
|
||||
|
||||
private static async Task<string> ReadBodyAsync(HttpResponseMessage response)
|
||||
|
||||
@@ -98,9 +98,11 @@ namespace OutlookAddin.Services
|
||||
{
|
||||
att.SaveAsFile(tempPath);
|
||||
var bytes = File.ReadAllBytes(tempPath);
|
||||
var name = att.FileName ?? "attachment.bin";
|
||||
env.Attachments.Add(new EspoAttachment
|
||||
{
|
||||
Name = att.FileName ?? "attachment.bin",
|
||||
Name = name,
|
||||
ContentType = GuessMimeType(name),
|
||||
ContentBase64 = Convert.ToBase64String(bytes)
|
||||
});
|
||||
}
|
||||
@@ -171,6 +173,70 @@ namespace OutlookAddin.Services
|
||||
return name;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Picks a sane MIME type from the file extension. EspoCRM's default
|
||||
/// attachment-upload config rejects "application/octet-stream" with
|
||||
/// "Not allowed file type", so we map common Office / image / archive
|
||||
/// extensions to canonical MIMEs before posting.
|
||||
/// </summary>
|
||||
private static string GuessMimeType(string fileName)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(fileName)) return "application/octet-stream";
|
||||
var ext = Path.GetExtension(fileName).ToLowerInvariant();
|
||||
switch (ext)
|
||||
{
|
||||
// Office (legacy + OOXML)
|
||||
case ".pdf": return "application/pdf";
|
||||
case ".doc": return "application/msword";
|
||||
case ".docx": return "application/vnd.openxmlformats-officedocument.wordprocessingml.document";
|
||||
case ".xls": return "application/vnd.ms-excel";
|
||||
case ".xlsx": return "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet";
|
||||
case ".ppt": return "application/vnd.ms-powerpoint";
|
||||
case ".pptx": return "application/vnd.openxmlformats-officedocument.presentationml.presentation";
|
||||
case ".rtf": return "application/rtf";
|
||||
case ".odt": return "application/vnd.oasis.opendocument.text";
|
||||
case ".ods": return "application/vnd.oasis.opendocument.spreadsheet";
|
||||
|
||||
// Text
|
||||
case ".txt": return "text/plain";
|
||||
case ".csv": return "text/csv";
|
||||
case ".log": return "text/plain";
|
||||
case ".htm":
|
||||
case ".html": return "text/html";
|
||||
case ".xml": return "application/xml";
|
||||
case ".json": return "application/json";
|
||||
|
||||
// Images
|
||||
case ".png": return "image/png";
|
||||
case ".jpg":
|
||||
case ".jpeg": return "image/jpeg";
|
||||
case ".gif": return "image/gif";
|
||||
case ".bmp": return "image/bmp";
|
||||
case ".tif":
|
||||
case ".tiff": return "image/tiff";
|
||||
case ".webp": return "image/webp";
|
||||
case ".svg": return "image/svg+xml";
|
||||
|
||||
// Archives
|
||||
case ".zip": return "application/zip";
|
||||
case ".rar": return "application/x-rar-compressed";
|
||||
case ".7z": return "application/x-7z-compressed";
|
||||
case ".gz": return "application/gzip";
|
||||
case ".tar": return "application/x-tar";
|
||||
|
||||
// Email
|
||||
case ".eml": return "message/rfc822";
|
||||
case ".msg": return "application/vnd.ms-outlook";
|
||||
|
||||
// Audio / video — rare in legal mail but cheap to handle
|
||||
case ".mp3": return "audio/mpeg";
|
||||
case ".wav": return "audio/wav";
|
||||
case ".mp4": return "video/mp4";
|
||||
|
||||
default: return "application/octet-stream";
|
||||
}
|
||||
}
|
||||
|
||||
private static T? SafeGet<T>(Func<T> getter) where T : class
|
||||
{
|
||||
try { return getter(); } catch { return null; }
|
||||
|
||||
Reference in New Issue
Block a user