fix(api): ZMS-89: harden list header parsing for get message endpoints

- salvage valid <URI> entries from partially malformed List-Unsubscribe
  values instead of discarding the whole header
- allow any URI scheme per RFC 2369 and non-ASCII characters in URIs
- keep unparseable values in the name property, the address property only
  ever contains a syntactically valid URI
- parse List-ID with the new module as well, addressparser returned the
  canonical bare <list-id> form as a display name with an empty address
- decode encoded word display names inside the module and do not run URI
  entries through punycode decoding
- guard punycode.toUnicode in tools.decodeAddresses, an invalid xn-- label
  in any address header crashed message detail responses with a RangeError
- normalize folding whitespace in repeated header values and comments
- run wildduck unit tests in grunt proto via the new unit mochaTest target
- clean up the extra message created by the List-Unsubscribe API test

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XBhfTgWZ9frXDrmenvdrx8

Author: andris9

Gruntfile.js

@@ -41,6 +41,20 @@ module.exports = function (grunt) {
                 // pop3 tests (do not require server/db)
                 src: ['test/pop3-*-test.js']
             },
+            unit: {
+                options: {
+                    reporter: 'spec'
+                },
+                // wildduck unit tests (do not require server/db)
+                src: [
+                    'test/checkrangequery-test.js',
+                    'test/create-decipher-test.js',
+                    'test/filtering-tools-test.js',
+                    'test/hibp-tools-test.js',
+                    'test/list-headers-test.js',
+                    'test/tools-test.js'
+                ]
+            },
             api: {
                 options: {
                     reporter: 'spec'
@@ -82,6 +96,6 @@ module.exports = function (grunt) {
     // Tasks
     grunt.registerTask('default', ['eslint', 'shell:server', 'wait:server', 'mochaTest', 'shell:server:kill']);
     grunt.registerTask('testonly', ['shell:server', 'wait:server', 'mochaTest', 'shell:server:kill']);
-    // proto: run all protocol-level tests (IMAP unit + POP3) without requiring MongoDB/Redis
-    grunt.registerTask('proto', ['mochaTest:imap-unit', 'mochaTest:pop3']);
+    // proto: run all protocol-level tests (IMAP unit + POP3 + unit) without requiring MongoDB/Redis
+    grunt.registerTask('proto', ['mochaTest:imap-unit', 'mochaTest:pop3', 'mochaTest:unit']);
 };

lib/api/messages.js

@@ -4,7 +4,6 @@ const config = require('@zone-eu/wild-config');
 const log = require('npmlog');
 const libmime = require('libmime');
 const Joi = require('joi');
-const addressparser = require('nodemailer/lib/addressparser');
 const MailComposer = require('nodemailer/lib/mail-composer');
 const { htmlToText } = require('html-to-text');
 const ObjectId = require('mongodb').ObjectId;
@@ -40,7 +39,7 @@ const { MsgEnvelope, MsgVerificationResults } = require('../schemas/response/mes
 const { successRes } = require('../schemas/response/general-schemas');
 const { mongopagingFindWrapper, mongopagingAggregateWrapper } = require('../mongopaging-find-wrapper');
 const { isEncryptedContentType } = require('../message-handler');
-const { parseListUnsubscribe } = require('../list-headers');
+const { parseListId, parseListUnsubscribe } = require('../list-headers');
 
 module.exports = (db, server, messageHandler, userHandler, storageHandler, settingsHandler) => {
     let maildrop = new Maildropper({
@@ -1587,17 +1586,11 @@ module.exports = (db, server, messageHandler, userHandler, storageHandler, setti
 
             let list;
             if (parsedHeader['list-id'] || parsedHeader['list-unsubscribe']) {
-                let listId = parsedHeader['list-id'];
-                if (listId) {
-                    listId = addressparser(listId.toString());
-                    tools.decodeAddresses(listId);
-                    listId = listId.shift();
-                }
+                let listId = parseListId(parsedHeader['list-id']) || undefined;
 
                 let listUnsubscribe = parsedHeader['list-unsubscribe'];
                 if (listUnsubscribe) {
                     listUnsubscribe = parseListUnsubscribe(listUnsubscribe);
-                    tools.decodeAddresses(listUnsubscribe);
                 }
 
                 list = {

lib/list-headers.js

@@ -1,22 +1,36 @@
 'use strict';
 
-const URI_REGEX = /^(?:https?|ftp|mailto):(?:[a-z0-9._~:/?#[\]@!$&'()*+,;=-]|%[a-f0-9]{2})+$/i;
-
-function skipFoldingWhitespace(value, start) {
-    let pos = start;
+const libmime = require('libmime');
+
+// RFC 2369 allows any URI scheme inside the angle brackets, so only require a generic
+// scheme prefix and reject whitespace, angle brackets and control characters
+const URI_REGEX = /^[a-z][a-z0-9+.-]*:[^\s<>]+$/i;
+
+// unfold and trim a header value, repeated header values are stored without unfolding
+function unfoldValue(value) {
+    return value
+        .toString()
+        .replace(/\s*\r?\n\s*/g, ' ')
+        .trim();
+}
 
-    while (pos < value.length) {
-        if (value[pos] === ' ' || value[pos] === '\t') {
-            pos++;
-            continue;
+// decode encoded words in a display name, keep the value as is on failure
+function decodeDisplayName(name) {
+    if (name.indexOf('=?') >= 0) {
+        try {
+            name = libmime.decodeWords(name);
+        } catch (err) {
+            // ignore, keep as is
         }
+    }
+    return name;
+}
 
-        if (value[pos] === '\r' && value[pos + 1] === '\n' && (value[pos + 2] === ' ' || value[pos + 2] === '\t')) {
-            pos += 2;
-            continue;
-        }
+function skipWhitespace(value, start) {
+    let pos = start;
 
-        break;
+    while (pos < value.length && (value[pos] === ' ' || value[pos] === '\t')) {
+        pos++;
     }
 
     return pos;
@@ -46,7 +60,7 @@ function readComment(value, start) {
             depth--;
             if (!depth) {
                 return {
-                    comment: comment.trim(),
+                    comment: comment.replace(/\s+/g, ' ').trim(),
                     pos: pos + 1
                 };
             }
@@ -61,7 +75,7 @@ function readComment(value, start) {
 }
 
 function readComments(value, start, comments) {
-    let pos = skipFoldingWhitespace(value, start);
+    let pos = skipWhitespace(value, start);
 
     while (value[pos] === '(') {
         const result = readComment(value, pos);
@@ -72,7 +86,7 @@ function readComments(value, start, comments) {
         if (result.comment) {
             comments.push(result.comment);
         }
-        pos = skipFoldingWhitespace(value, result.pos);
+        pos = skipWhitespace(value, result.pos);
     }
 
     return pos;
@@ -107,7 +121,7 @@ function parseHeaderValue(value) {
 
         entries.push({
             address,
-            name: comments.join(' ')
+            name: decodeDisplayName(comments.join(' '))
         });
 
         if (pos === value.length) {
@@ -123,6 +137,24 @@ function parseHeaderValue(value) {
     return false;
 }
 
+// last resort scan for valid <URI> segments inside an otherwise malformed value
+function salvageUris(value) {
+    const entries = [];
+
+    const re = /<([^<>]+)>/g;
+    let match;
+    while ((match = re.exec(value))) {
+        if (URI_REGEX.test(match[1])) {
+            entries.push({
+                address: match[1],
+                name: ''
+            });
+        }
+    }
+
+    return entries;
+}
+
 function parseListUnsubscribe(value) {
     const entries = [];
 
@@ -131,23 +163,63 @@ function parseListUnsubscribe(value) {
             continue;
         }
 
-        const source = headerValue.toString();
+        const source = unfoldValue(headerValue);
         if (!source) {
             continue;
         }
 
-        const parsed = parseHeaderValue(source);
-        entries.push(
-            ...(parsed || [
-                {
-                    address: source,
-                    name: ''
-                }
-            ])
-        );
+        // strict RFC 2369 parse first, then try to salvage valid <URI> entries from a malformed value
+        const parsed = parseHeaderValue(source) || salvageUris(source);
+        if (parsed.length) {
+            entries.push(...parsed);
+        } else {
+            // No valid URI found. Keep the raw value as the display name, the address property
+            // only ever contains a syntactically valid URI. Consumers must still restrict allowed
+            // schemes before using the address as a link target
+            entries.push({
+                address: '',
+                name: decodeDisplayName(source)
+            });
+        }
     }
 
     return entries;
 }
 
-module.exports = { parseListUnsubscribe };
+function parseListId(value) {
+    if (Array.isArray(value)) {
+        // RFC 2919 allows a single List-ID header only, ignore the rest
+        value = value[0];
+    }
+
+    if (!value) {
+        return false;
+    }
+
+    const source = unfoldValue(value);
+    if (!source) {
+        return false;
+    }
+
+    // RFC 2919: optional display name (phrase) followed by "<" list-id ">"
+    const match = /^([^<>]*)<([^<>\s]+)>$/.exec(source);
+    if (!match) {
+        // not in the expected format, keep the raw value as the display name
+        return {
+            address: '',
+            name: decodeDisplayName(source)
+        };
+    }
+
+    let name = match[1].trim();
+    if (name.length > 1 && name[0] === '"' && name[name.length - 1] === '"') {
+        name = name.slice(1, -1).replace(/\\(.)/g, '$1').trim();
+    }
+
+    return {
+        address: match[2],
+        name: decodeDisplayName(name)
+    };
+}
+
+module.exports = { parseListId, parseListUnsubscribe };

lib/tools.js

@@ -184,9 +184,13 @@ function decodeAddresses(addresses) {
             }
         }
         if (/@xn--/.test(address.address)) {
-            address.address =
-                address.address.substr(0, address.address.lastIndexOf('@') + 1) +
-                punycode.toUnicode(address.address.substr(address.address.lastIndexOf('@') + 1));
+            try {
+                address.address =
+                    address.address.substr(0, address.address.lastIndexOf('@') + 1) +
+                    punycode.toUnicode(address.address.substr(address.address.lastIndexOf('@') + 1));
+            } catch (E) {
+                //ignore, keep as is
+            }
         }
         if (address.group) {
             decodeAddresses(address.group);

test/api/messages-test.js

@@ -590,7 +590,7 @@ describe('Messages tests', function () {
         queryThread = keywordMessageDetails.body.thread;
     });
 
-    it('should GET /users/:user/mailboxes/:mailbox/messages/:message preserve a malformed List-Unsubscribe value', async () => {
+    it('should GET /users/:user/mailboxes/:mailbox/messages/:message salvage a malformed List-Unsubscribe value', async () => {
         const listUnsubscribe = 'Unsubscribe here <mailto:unsub@example.com>';
         const messageResponse = await server
             .post(`/users/${user}/mailboxes/${testMailbox}/messages`)
@@ -609,10 +609,13 @@ describe('Messages tests', function () {
 
         expect(messageData.body.list.unsubscribe).to.deep.equal([
             {
-                address: listUnsubscribe,
+                address: 'mailto:unsub@example.com',
                 name: ''
             }
         ]);
+
+        // remove the extra message so that later tests see an unchanged mailbox
+        await server.delete(`/users/${user}/mailboxes/${testMailbox}/messages/${messageResponse.body.message.id}`).expect(200);
     });
 
     it('should POST /users/:user/mailboxes/:mailbox/messages/:message/submit expect failure / recipient pre-check counts all recipients', async () => {