for errors during maildir/mbox zip/tgz import in account page, return http 400 for user errors (e.g. bad file format) and show the error message

This commit is contained in:
Mechiel Lukkien 2024-01-05 11:31:05 +01:00
parent 62db2af846
commit ac8256feb6
No known key found for this signature in database
4 changed files with 20 additions and 16 deletions

View file

@ -306,10 +306,14 @@ func handle(apiHandler http.Handler, isForwarded bool, w http.ResponseWriter, r
http.Error(w, "500 - internal server error - "+err.Error(), http.StatusInternalServerError) http.Error(w, "500 - internal server error - "+err.Error(), http.StatusInternalServerError)
return return
} }
token, err := importStart(log, accName, tmpf, skipMailboxPrefix) token, isUserError, err := importStart(log, accName, tmpf, skipMailboxPrefix)
if err != nil { if err != nil {
log.Errorx("starting import", err) log.Errorx("starting import", err, slog.Bool("usererror", isUserError))
http.Error(w, "500 - internal server error - "+err.Error(), http.StatusInternalServerError) if isUserError {
http.Error(w, "400 - bad request - "+err.Error(), http.StatusBadRequest)
} else {
http.Error(w, "500 - internal server error - "+err.Error(), http.StatusInternalServerError)
}
return return
} }
tmpf = nil // importStart is now responsible for cleanup. tmpf = nil // importStart is now responsible for cleanup.

View file

@ -973,7 +973,7 @@ const index = async () => {
xhr.addEventListener('load', () => { xhr.addEventListener('load', () => {
console.log('upload done', { xhr: xhr, status: xhr.status }); console.log('upload done', { xhr: xhr, status: xhr.status });
if (xhr.status !== 200) { if (xhr.status !== 200) {
reject({ message: 'status ' + xhr.status }); reject({ message: xhr.status === 400 || xhr.status === 500 ? xhr.responseText : 'status ' + xhr.status });
return; return;
} }
let resp; let resp;

View file

@ -458,7 +458,7 @@ const index = async () => {
xhr.addEventListener('load', () => { xhr.addEventListener('load', () => {
console.log('upload done', {xhr: xhr, status: xhr.status}) console.log('upload done', {xhr: xhr, status: xhr.status})
if (xhr.status !== 200) { if (xhr.status !== 200) {
reject({message: 'status '+xhr.status}) reject({message: xhr.status === 400 || xhr.status === 500 ? xhr.responseText : 'status '+xhr.status})
return return
} }
let resp: api.ImportProgress let resp: api.ImportProgress

View file

@ -200,7 +200,7 @@ type importStep struct {
// importStart prepare the import and launches the goroutine to actually import. // importStart prepare the import and launches the goroutine to actually import.
// importStart is responsible for closing f and removing f. // importStart is responsible for closing f and removing f.
func importStart(log mlog.Log, accName string, f *os.File, skipMailboxPrefix string) (string, error) { func importStart(log mlog.Log, accName string, f *os.File, skipMailboxPrefix string) (string, bool, error) {
defer func() { defer func() {
if f != nil { if f != nil {
store.CloseRemoveTempFile(log, f, "upload for import") store.CloseRemoveTempFile(log, f, "upload for import")
@ -209,12 +209,12 @@ func importStart(log mlog.Log, accName string, f *os.File, skipMailboxPrefix str
buf := make([]byte, 16) buf := make([]byte, 16)
if _, err := cryptrand.Read(buf); err != nil { if _, err := cryptrand.Read(buf); err != nil {
return "", err return "", false, err
} }
token := fmt.Sprintf("%x", buf) token := fmt.Sprintf("%x", buf)
if _, err := f.Seek(0, 0); err != nil { if _, err := f.Seek(0, 0); err != nil {
return "", fmt.Errorf("seek to start of file: %v", err) return "", false, fmt.Errorf("seek to start of file: %v", err)
} }
// Recognize file format. // Recognize file format.
@ -223,12 +223,12 @@ func importStart(log mlog.Log, accName string, f *os.File, skipMailboxPrefix str
magicGzip := []byte{0x1f, 0x8b} magicGzip := []byte{0x1f, 0x8b}
magic := make([]byte, 4) magic := make([]byte, 4)
if _, err := f.ReadAt(magic, 0); err != nil { if _, err := f.ReadAt(magic, 0); err != nil {
return "", fmt.Errorf("detecting file format: %v", err) return "", true, fmt.Errorf("detecting file format: %v", err)
} }
if bytes.Equal(magic, magicZip) { if bytes.Equal(magic, magicZip) {
iszip = true iszip = true
} else if !bytes.Equal(magic[:2], magicGzip) { } else if !bytes.Equal(magic[:2], magicGzip) {
return "", fmt.Errorf("file is not a zip or gzip file") return "", true, fmt.Errorf("file is not a zip or gzip file")
} }
var zr *zip.Reader var zr *zip.Reader
@ -236,23 +236,23 @@ func importStart(log mlog.Log, accName string, f *os.File, skipMailboxPrefix str
if iszip { if iszip {
fi, err := f.Stat() fi, err := f.Stat()
if err != nil { if err != nil {
return "", fmt.Errorf("stat temporary import zip file: %v", err) return "", false, fmt.Errorf("stat temporary import zip file: %v", err)
} }
zr, err = zip.NewReader(f, fi.Size()) zr, err = zip.NewReader(f, fi.Size())
if err != nil { if err != nil {
return "", fmt.Errorf("opening zip file: %v", err) return "", true, fmt.Errorf("opening zip file: %v", err)
} }
} else { } else {
gzr, err := gzip.NewReader(f) gzr, err := gzip.NewReader(f)
if err != nil { if err != nil {
return "", fmt.Errorf("gunzip: %v", err) return "", true, fmt.Errorf("gunzip: %v", err)
} }
tr = tar.NewReader(gzr) tr = tar.NewReader(gzr)
} }
acc, err := store.OpenAccount(log, accName) acc, err := store.OpenAccount(log, accName)
if err != nil { if err != nil {
return "", fmt.Errorf("open acount: %v", err) return "", false, fmt.Errorf("open acount: %v", err)
} }
acc.Lock() // Not using WithWLock because importMessage is responsible for unlocking. acc.Lock() // Not using WithWLock because importMessage is responsible for unlocking.
@ -261,7 +261,7 @@ func importStart(log mlog.Log, accName string, f *os.File, skipMailboxPrefix str
acc.Unlock() acc.Unlock()
xerr := acc.Close() xerr := acc.Close()
log.Check(xerr, "closing account") log.Check(xerr, "closing account")
return "", fmt.Errorf("start transaction: %v", err) return "", false, fmt.Errorf("start transaction: %v", err)
} }
// Ensure token is registered before returning, with context that can be canceled. // Ensure token is registered before returning, with context that can be canceled.
@ -272,7 +272,7 @@ func importStart(log mlog.Log, accName string, f *os.File, skipMailboxPrefix str
go importMessages(ctx, log.WithCid(mox.Cid()), token, acc, tx, zr, tr, f, skipMailboxPrefix) go importMessages(ctx, log.WithCid(mox.Cid()), token, acc, tx, zr, tr, f, skipMailboxPrefix)
f = nil // importMessages is now responsible for closing and removing. f = nil // importMessages is now responsible for closing and removing.
return token, nil return token, false, nil
} }
// importMessages imports the messages from zip/tgz file f. // importMessages imports the messages from zip/tgz file f.