Skip to content

Commit

Permalink
email: validate addresses following the HTML specs
Browse files Browse the repository at this point in the history
If we are going to try and validate email addresses, at least do it by
following some existing specification and document it.

It remains on app developpers to:
 * make sure their addresses are ASCII-only, by converting international
   labels to Punycode
 * remove anything but the addresses, `Some Name <[email protected]>` is
   not accepted

[smcv: Fix merge conflicts in tests/test_email.py]
Co-authored-by: Simon McVittie <[email protected]>
  • Loading branch information
nicolas-guichard authored and smcv committed Sep 14, 2023
1 parent 83953d2 commit a2ccb56
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 3 deletions.
6 changes: 4 additions & 2 deletions data/org.freedesktop.portal.Email.xml
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,15 @@
<varlistentry>
<term>address s</term>
<listitem><para>
The email address to send to.
The email address to send to. Must conform to the HTML5 definition of
<ulink url="https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address">valid email address</ulink>.
</para></listitem>
</varlistentry>
<varlistentry>
<term>addresses as</term>
<listitem><para>
Email addresses to send to. This will be used in addition to address.
Email addresses to send to. This will be used in addition to address and must
pass the same validation.
</para><para>
This option was added in version 3.
</para></listitem>
Expand Down
6 changes: 5 additions & 1 deletion src/email.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,11 @@ compose_email_done (GObject *source,
static gboolean
is_valid_email (const char *string)
{
return g_regex_match_simple ("^\\w+([-+.]\\w+)*@\\w+([-.]\\w+)*\\.\\w+([-.]\\w+)*$", string, 0, 0);
// Regex proposed by the W3C at https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
return g_regex_match_simple ("^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+"
"@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?"
"(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$",
string, 0, 0);
}

static gboolean
Expand Down
42 changes: 42 additions & 0 deletions tests/email.c
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,48 @@ test_email_address (void)
g_main_context_iteration (NULL, TRUE);
}

/* test that punycode-encoded email addresses pass validation
*/
void
test_email_punycode_address (void)
{
g_autoptr(XdpPortal) portal = NULL;
g_autoptr(GKeyFile) keyfile = NULL;
g_autoptr(GError) error = NULL;
g_autofree char *path = NULL;
const char *addresses[2] = { "[email protected]", NULL };

keyfile = g_key_file_new ();

g_key_file_set_string (keyfile, "input", "address", "[email protected]");
g_key_file_set_string (keyfile, "input", "subject", "Re: portal tests");
g_key_file_set_string (keyfile, "input", "body", "To ASCII and beyond");

g_key_file_set_integer (keyfile, "backend", "delay", 0);
g_key_file_set_integer (keyfile, "backend", "response", 0);
g_key_file_set_integer (keyfile, "result", "response", 0);

path = g_build_filename (outdir, "email", NULL);
g_key_file_save_to_file (keyfile, path, &error);
g_assert_no_error (error);

portal = xdp_portal_new ();

got_info = 0;
xdp_portal_compose_email (portal, NULL,
addresses, NULL, NULL,
"Re: portal tests",
"To ASCII and beyond",
NULL,
0,
NULL,
email_cb,
keyfile);

while (!got_info)
g_main_context_iteration (NULL, TRUE);
}

/* test that an invalid subject triggers an error
*/
void
Expand Down
1 change: 1 addition & 0 deletions tests/email.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

void test_email_basic (void);
void test_email_address (void);
void test_email_punycode_address (void);
void test_email_subject (void);
void test_email_delay (void);
void test_email_cancel (void);
Expand Down
1 change: 1 addition & 0 deletions tests/test-portals.c
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ main (int argc, char **argv)
g_test_add_func ("/portal/email/cancel", test_email_cancel);
g_test_add_func ("/portal/email/close", test_email_close);
g_test_add_func ("/portal/email/address", test_email_address);
g_test_add_func ("/portal/email/punycode_address", test_email_punycode_address);
g_test_add_func ("/portal/email/subject", test_email_subject);
g_test_add_func ("/portal/email/parallel", test_email_parallel);

Expand Down
28 changes: 28 additions & 0 deletions tests/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,34 @@ def test_email_address(self, portal_mock):
method_calls = portal_mock.mock_interface.GetMethodCalls("ComposeEmail")
assert len(method_calls) == 0

def test_email_punycode_address(self, portal_mock):
addresses = ["[email protected]"]
subject = "Re: portal tests"
body = "To ASCII and beyond"

request = portal_mock.create_request()
options = {
"addresses": addresses,
"subject": subject,
"body": body,
}
response = request.call(
"ComposeEmail",
parent_window="",
options=options,
)

assert response.response == 0

# Check the impl portal was called with the right args
method_calls = portal_mock.mock_interface.GetMethodCalls("ComposeEmail")
assert len(method_calls) > 0
_, args = method_calls[-1]
assert args[2] == "" # parent window
assert args[3]["addresses"] == addresses
assert args[3]["subject"] == subject
assert args[3]["body"] == body

def test_email_subject_multiline(self, portal_mock):
"""test that an multiline subject triggers an error"""

Expand Down

0 comments on commit a2ccb56

Please sign in to comment.