Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Zms 90 #503

Merged
merged 9 commits into from
Sep 28, 2023
Merged

Zms 90 #503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
39e23ea
mailbox-handler create() returns errors, code and http status codes c…
NickOvt Sep 21, 2023
4817593
mailboxes handler now correctly return errors, error codes and approp…
NickOvt Sep 21, 2023
45a5829
mailbox-handler.js codes fix + tests fix in mailboxes-test.js
NickOvt Sep 22, 2023
3887cfb
mailbox-handler.js correct error handling for update and rename
NickOvt Sep 22, 2023
02edc3c
fix mailboxes.js http status part for PUT request, fix appropriate tests
NickOvt Sep 22, 2023
d39634a
update responseWrapper, remove unnecessary try catches, fix mailbox-h…
NickOvt Sep 25, 2023
ce73fbb
fix imap-core create
NickOvt Sep 27, 2023
f206ac9
reverted 405 errors to 400
NickOvt Sep 28, 2023
19a9d44
fix tests
NickOvt Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion imap-core/lib/commands/create.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ module.exports = {
// do not return actual error to user
return callback(null, {
response: 'NO',
code: 'TEMPFAIL'
code: err.code || 'TEMPFAIL'
});
}

Expand Down
59 changes: 6 additions & 53 deletions lib/api/mailboxes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,24 +296,10 @@ module.exports = (db, server, mailboxHandler) => {
}

let status, id;
try {
let data = await createMailbox(user, path, opts);
status = data.status;
id = data.id;
} catch (err) {
res.status(500); // TODO: use response code specific status
return res.json({
error: err.message,
code: err.code
});
}

if (typeof status === 'string') {
res.status(500);
return res.json({
error: 'Mailbox creation failed with code ' + status
});
}
let data = await createMailbox(user, path, opts);
status = data.status;
id = data.id;

return res.json({
success: !!status,
Expand All @@ -326,7 +312,7 @@ module.exports = (db, server, mailboxHandler) => {
'/users/:user/mailboxes/:mailbox',
tools.responseWrapper(async (req, res) => {
res.charSet('utf-8');
const schema = Joi.object().keys({
const schema = Joi.object({
user: Joi.string().hex().lowercase().length(24).required(),
mailbox: Joi.string().hex().lowercase().length(24).allow('resolve').required(),
path: Joi.string().regex(/\/{2,}|\/$/, { invert: true }),
Expand Down Expand Up @@ -506,23 +492,7 @@ module.exports = (db, server, mailboxHandler) => {
});
}

let status;
try {
status = await updateMailbox(user, mailbox, updates);
} catch (err) {
res.status(500); // TODO: use response code specific status
return res.json({
error: err.message,
code: err.code
});
}

if (typeof status === 'string') {
res.status(500);
return res.json({
error: 'Mailbox update failed with code ' + status
});
}
await updateMailbox(user, mailbox, updates);

return res.json({
success: true
Expand Down Expand Up @@ -566,24 +536,7 @@ module.exports = (db, server, mailboxHandler) => {
let user = new ObjectId(result.value.user);
let mailbox = new ObjectId(result.value.mailbox);

let status;
try {
status = await deleteMailbox(user, mailbox);
} catch (err) {
res.status(500); // TODO: use response code specific status
return res.json({
error: err.message,
code: err.code
});
}

if (typeof status === 'string') {
res.status(500); // TODO: use response code specific status
return res.json({
error: 'Mailbox deletion failed with code ' + status,
code: status
});
}
await deleteMailbox(user, mailbox);

return res.json({
success: true
Expand Down
55 changes: 44 additions & 11 deletions lib/mailbox-handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ class MailboxHandler {
return callback(err);
}
if (mailboxData) {
return callback(null, 'ALREADYEXISTS');
const err = new Error('Mailbox creation failed with code MailboxAlreadyExists');
err.code = 'ALREADYEXISTS';
err.responseCode = 400;
return callback(err, 'ALREADYEXISTS');
}

this.users.collection('users').findOne(
Expand All @@ -50,7 +53,10 @@ class MailboxHandler {
}

if (!userData) {
return callback(new Error('User not found'));
const err = new Error('This user does not exist');
err.code = 'UserNotFound';
err.responseCode = 404;
return callback(err, 'UserNotFound');
}

mailboxData = {
Expand All @@ -74,7 +80,10 @@ class MailboxHandler {
this.database.collection('mailboxes').insertOne(mailboxData, { writeConcern: 'majority' }, (err, r) => {
if (err) {
if (err.code === 11000) {
return callback(null, 'ALREADYEXISTS');
const err = new Error('Mailbox creation failed with code MailboxAlreadyExists');
err.code = 'ALREADYEXISTS';
err.responseCode = 400;
return callback(err, 'ALREADYEXISTS');
}
return callback(err);
}
Expand Down Expand Up @@ -116,10 +125,16 @@ class MailboxHandler {
return callback(err);
}
if (!mailboxData) {
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox update failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}
if (mailboxData.path === 'INBOX' || mailboxData.hidden) {
return callback(null, 'CANNOT');
const err = new Error('Mailbox update failed with code DisallowedMailboxMethod');
err.code = 'CANNOT';
err.responseCode = 400;
return callback(err, 'CANNOT');
}
this.database.collection('mailboxes').findOne(
{
Expand All @@ -131,7 +146,10 @@ class MailboxHandler {
return callback(err);
}
if (existing) {
return callback(null, 'ALREADYEXISTS');
const err = new Error('Mailbox rename failed with code MailboxAlreadyExists');
err.code = 'ALREADYEXISTS';
err.responseCode = 400;
return callback(err, 'ALREADYEXISTS');
}

let $set = { path: newname };
Expand All @@ -157,7 +175,10 @@ class MailboxHandler {

if (!item || !item.value) {
// was not able to acquire a lock
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox update failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}

publish(this.redis, {
Expand Down Expand Up @@ -201,10 +222,16 @@ class MailboxHandler {
return callback(err);
}
if (!mailboxData) {
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox deletion failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}
if (mailboxData.specialUse || mailboxData.path === 'INBOX' || mailboxData.hidden) {
return callback(null, 'CANNOT');
const err = new Error('Mailbox deletion failed with code DisallowedMailboxMethod');
err.code = 'CANNOT';
err.responseCode = 400;
return callback(err, 'CANNOT');
}

this.database.collection('mailboxes').deleteOne(
Expand Down Expand Up @@ -338,7 +365,10 @@ class MailboxHandler {
return callback(err);
}
if (!mailboxData) {
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox update failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}

if (updates.path && updates.path !== mailboxData.path) {
Expand Down Expand Up @@ -373,7 +403,10 @@ class MailboxHandler {
}

if (!item || !item.value) {
return callback(null, 'NONEXISTENT');
const err = new Error('Mailbox update failed with code NoSuchMailbox');
err.code = 'NONEXISTENT';
err.responseCode = 404;
return callback(err, 'NONEXISTENT');
}

return callback(null, true);
Expand Down
4 changes: 4 additions & 0 deletions lib/tools.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,10 @@ module.exports = {
err.responseCode = err.responseCode || 404;
err.code = 'NoSuchMailbox';
break;
case 'CANNOT':
err.responseCode = err.responseCode || 400;
err.code = 'DisallowedMailboxMethod';
break;
}

if (err.responseCode) {
Expand Down
68 changes: 24 additions & 44 deletions test/api/mailboxes-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ describe('Mailboxes tests', function () {
expect(response.body.error).to.not.be.empty;
});

// TODO: rewrite when mailboxes.js gets updated status codes
it('should POST /users/{user}/mailboxes expect failure / user not found', async () => {
const response = await server
.post(`/users/${'0'.repeat(24)}/mailboxes`)
.send({ path: '/cool2/path2/whatisthis346/cool-drink', hidden: false, retention: 0 })
.expect(500);
.expect(404);

expect(response.body.error).to.be.equal('User not found');
expect(response.body.error).to.be.equal('This user does not exist');
expect(response.body.code).to.be.equal('UserNotFound');
});

it('should POST /users/{user}/mailboxes expect failure / path is missing', async () => {
Expand Down Expand Up @@ -228,30 +228,21 @@ describe('Mailboxes tests', function () {
expect(response3.body.code).to.be.equal('InputValidationError');
expect(response3.body.error).to.be.not.empty;

const response4 = await server
.get(`/users/${user}/mailboxes/${'-'.repeat(24)}`)

.expect(400);
const response4 = await server.get(`/users/${user}/mailboxes/${'-'.repeat(24)}`).expect(400);

expect(response4.body.code).to.be.equal('InputValidationError');
expect(response4.body.error).to.be.not.empty;
});

it('should GET /users/{user}/mailboxes/{mailbox} expect failure / mailbox not found', async () => {
const response = await server
.get(`/users/${user}/mailboxes/${'0'.repeat(24)}`)

.expect(404);
const response = await server.get(`/users/${user}/mailboxes/${'0'.repeat(24)}`).expect(404);

expect(response.body.code).to.be.equal('NoSuchMailbox');
expect(response.body.error).to.be.equal('This mailbox does not exist');
});

it('should GET /users/{user}/mailboxes/{mailbox} expect failure / user not found', async () => {
const response = await server
.get(`/users/${'0'.repeat(24)}/mailboxes`)

.expect(404);
const response = await server.get(`/users/${'0'.repeat(24)}/mailboxes`).expect(404);

expect(response.body.error).to.be.equal('This user does not exist');
expect(response.body.code).to.be.equal('UserNotFound');
Expand Down Expand Up @@ -335,9 +326,9 @@ describe('Mailboxes tests', function () {
const response = await server
.put(`/users/${user}/mailboxes/${'0'.repeat(24)}`)
.send({ path: 'newPath' })
.expect(500);
.expect(404);

expect(response.body.error).to.be.equal('Mailbox update failed with code NONEXISTENT');
expect(response.body.error).to.be.equal('Mailbox update failed with code NoSuchMailbox');
});

it('should PUT /users/{user}/mailboxes/{mailbox} expect failure / user not found', async () => {
Expand All @@ -346,18 +337,15 @@ describe('Mailboxes tests', function () {
const response = await server
.put(`/users/${'0'.repeat(24)}/mailboxes/${mailboxes.body.results.at(-1).id}`)
.send({ path: 'newPath' })
.expect(500);
.expect(404);

expect(response.body.error).to.be.equal('Mailbox update failed with code NONEXISTENT');
expect(response.body.error).to.be.equal('Mailbox update failed with code NoSuchMailbox');
});

it('should PUT /users/{user}/mailboxes/{mailbox} expect failure / nothing was changed', async () => {
const mailboxes = await server.get(`/users/${user}/mailboxes`).expect(200);

const response = await server
.put(`/users/${user}/mailboxes/${mailboxes.body.results.at(-1).id}`)

.expect(400);
const response = await server.put(`/users/${user}/mailboxes/${mailboxes.body.results.at(-1).id}`).expect(400);

expect(response.body.error).to.be.equal('Nothing was changed');
});
Expand All @@ -367,9 +355,9 @@ describe('Mailboxes tests', function () {

const inboxMailbox = mailboxes.body.results.find(el => el.path === 'INBOX');

const response = await server.put(`/users/${user}/mailboxes/${inboxMailbox.id}`).send({ path: 'newPath/folder123' }).expect(500);
const response = await server.put(`/users/${user}/mailboxes/${inboxMailbox.id}`).send({ path: 'newPath/folder123' }).expect(400);

expect(response.body.error).to.be.equal('Mailbox update failed with code CANNOT');
expect(response.body.error).to.be.equal('Mailbox update failed with code DisallowedMailboxMethod');
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect success', async () => {
Expand All @@ -385,10 +373,10 @@ describe('Mailboxes tests', function () {
it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / protected path', async () => {
const mailboxes = await server.get(`/users/${user}/mailboxes`).expect(200);

const response = await server.del(`/users/${user}/mailboxes/${mailboxes.body.results[0].id}`).expect(500);
const response = await server.del(`/users/${user}/mailboxes/${mailboxes.body.results[0].id}`).expect(400);

expect(response.body.error).to.be.equal('Mailbox deletion failed with code CANNOT');
expect(response.body.code).to.be.equal('CANNOT');
expect(response.body.error).to.be.equal('Mailbox deletion failed with code DisallowedMailboxMethod');
expect(response.body.code).to.be.equal('DisallowedMailboxMethod');
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / incorrect params', async () => {
Expand Down Expand Up @@ -422,36 +410,28 @@ describe('Mailboxes tests', function () {
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / mailbox not found', async () => {
const response = await server
.del(`/users/${user}/mailboxes/${'0'.repeat(24)}`)

.expect(500);

console.log(response.body);
const response = await server.del(`/users/${user}/mailboxes/${'0'.repeat(24)}`).expect(404);

expect(response.body.error).to.be.equal('Mailbox deletion failed with code NONEXISTENT');
expect(response.body.code).to.be.equal('NONEXISTENT');
expect(response.body.error).to.be.equal('Mailbox deletion failed with code NoSuchMailbox');
expect(response.body.code).to.be.equal('NoSuchMailbox');
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / user not found', async () => {
const mailboxes = await server.get(`/users/${user}/mailboxes`).expect(200);

const response = await server
.del(`/users/${'0'.repeat(24)}/mailboxes/${mailboxes.body.results[0].id}`)

.expect(500);
const response = await server.del(`/users/${'0'.repeat(24)}/mailboxes/${mailboxes.body.results[0].id}`).expect(404);

expect(response.body.error).to.be.equal('Mailbox deletion failed with code NONEXISTENT');
expect(response.body.error).to.be.equal('Mailbox deletion failed with code NoSuchMailbox');
});

it('should DELETE /users/{user}/mailboxes/{mailbox} expect failure / cannot delete inbox', async () => {
const mailboxes = await server.get(`/users/${user}/mailboxes`).expect(200);

const inboxMailbox = mailboxes.body.results.find(el => el.path === 'INBOX');

const response = await server.del(`/users/${user}/mailboxes/${inboxMailbox.id}`).expect(500);
const response = await server.del(`/users/${user}/mailboxes/${inboxMailbox.id}`).expect(400);

expect(response.body.error).to.be.equal('Mailbox deletion failed with code CANNOT');
expect(response.body.code).to.be.equal('CANNOT');
expect(response.body.error).to.be.equal('Mailbox deletion failed with code DisallowedMailboxMethod');
expect(response.body.code).to.be.equal('DisallowedMailboxMethod');
});
});
Loading