Changeset View
Standalone View
src/kemailaddress.cpp
Show All 15 Lines | 1 | /* | |||
---|---|---|---|---|---|
16 | the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, | 16 | the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, | ||
17 | Boston, MA 02110-1301, USA. | 17 | Boston, MA 02110-1301, USA. | ||
18 | */ | 18 | */ | ||
19 | 19 | | |||
20 | #include "kemailaddress.h" | 20 | #include "kemailaddress.h" | ||
21 | #include "kcodecs.h" | 21 | #include "kcodecs.h" | ||
22 | #include "kcodecs_debug.h" | 22 | #include "kcodecs_debug.h" | ||
23 | 23 | | |||
24 | #include <QUrl> | | |||
25 | | ||||
26 | #include <QRegExp> | | |||
27 | #include <QByteArray> | 24 | #include <QByteArray> | ||
25 | #include <QRegularExpression> | ||||
26 | #include <QUrl> | ||||
28 | 27 | | |||
29 | using namespace KEmailAddress; | 28 | using namespace KEmailAddress; | ||
30 | 29 | | |||
31 | //----------------------------------------------------------------------------- | 30 | //----------------------------------------------------------------------------- | ||
32 | QStringList KEmailAddress::splitAddressList(const QString &aStr) | 31 | QStringList KEmailAddress::splitAddressList(const QString &aStr) | ||
33 | { | 32 | { | ||
34 | // Features: | 33 | // Features: | ||
35 | // - always ignores quoted characters | 34 | // - always ignores quoted characters | ||
▲ Show 20 Lines • Show All 565 Lines • ▼ Show 20 Line(s) | 590 | { | |||
601 | 600 | | |||
602 | // Both of these parts must be non empty | 601 | // Both of these parts must be non empty | ||
603 | // after all we cannot have emails like: | 602 | // after all we cannot have emails like: | ||
604 | // @kde.org, or foo@ | 603 | // @kde.org, or foo@ | ||
605 | if (localPart.isEmpty() || domainPart.isEmpty()) { | 604 | if (localPart.isEmpty() || domainPart.isEmpty()) { | ||
606 | return false; | 605 | return false; | ||
607 | } | 606 | } | ||
608 | 607 | | |||
609 | bool tooManyAtsFlag = false; | 608 | bool tooManyAtsFlag = false; | ||
dfaure: [pre-existing: this bool is never set to true....] | |||||
610 | bool inQuotedString = false; | 609 | bool inQuotedString = false; | ||
611 | int atCount = localPart.count(QLatin1Char('@')); | 610 | int atCount = localPart.count(QLatin1Char('@')); | ||
612 | 611 | | |||
613 | unsigned int strlen = localPart.length(); | 612 | unsigned int strlen = localPart.length(); | ||
614 | for (unsigned int index = 0; index < strlen; index++) { | 613 | for (unsigned int index = 0; index < strlen; index++) { | ||
615 | switch (localPart[ index ].toLatin1()) { | 614 | switch (localPart[ index ].toLatin1()) { | ||
616 | case '"' : | 615 | case '"' : | ||
617 | inQuotedString = !inQuotedString; | 616 | inQuotedString = !inQuotedString; | ||
618 | break; | 617 | break; | ||
619 | case '@' : | 618 | case '@' : | ||
620 | if (inQuotedString) { | 619 | if (inQuotedString) { | ||
621 | --atCount; | 620 | --atCount; | ||
622 | if (atCount == 0) { | 621 | if (atCount == 0) { | ||
623 | tooManyAtsFlag = false; | 622 | tooManyAtsFlag = false; | ||
624 | } | 623 | } | ||
625 | } | 624 | } | ||
626 | break; | 625 | break; | ||
627 | } | 626 | } | ||
628 | } | 627 | } | ||
629 | 628 | | |||
630 | QString addrRx; | 629 | QString addrRx; | ||
631 | 630 | | |||
632 | if (localPart[ 0 ] == QLatin1Char('\"') || localPart[ localPart.length() - 1 ] == QLatin1Char('\"')) { | 631 | if (localPart[ 0 ] == QLatin1Char('\"') || localPart[ localPart.length() - 1 ] == QLatin1Char('\"')) { | ||
Message from Giuseppe D'Angelo (author of QRegularExpression) : if you can depend from Qt 5.12, there's QRegularExpression::anchoredPattern instead of manual wrapping in \A and \z We can't require 5.12 yet but this could be a TODO to simplify this code slightly at some point. dfaure: Message from Giuseppe D'Angelo (author of QRegularExpression) : if you can depend from Qt 5.12… | |||||
Yep, I added the Qt 5.12 TODO in some QRegExp porting diffs but not all (kind of got boring (lots of QRegExp::exactMatch()), but that's not an excuse to be lazy and make it harder for the next guy who modifies that code.... :)). ahmadsamir: Yep, I added the Qt 5.12 TODO in some QRegExp porting diffs but not all (kind of got boring… | |||||
633 | addrRx = QStringLiteral("\"[a-zA-Z@]*[\\w.@-]*[a-zA-Z0-9@]\"@"); | 632 | addrRx = QStringLiteral("\"[a-zA-Z@]*[\\w.@-]*[a-zA-Z0-9@]\"@"); | ||
634 | } else { | 633 | } else { | ||
635 | addrRx = QStringLiteral("[a-zA-Z]*[~|{}`\\^?=/+*'&%$#!_\\w.-]*[~|{}`\\^?=/+*'&%$#!_a-zA-Z0-9-]@"); | 634 | addrRx = QStringLiteral("[a-zA-Z]*[~|{}`\\^?=/+*'&%$#!_\\w.-]*[~|{}`\\^?=/+*'&%$#!_a-zA-Z0-9-]@"); | ||
636 | } | 635 | } | ||
637 | if (domainPart[ 0 ] == QLatin1Char('[') || domainPart[ domainPart.length() - 1 ] == QLatin1Char(']')) { | 636 | if (domainPart[ 0 ] == QLatin1Char('[') || domainPart[ domainPart.length() - 1 ] == QLatin1Char(']')) { | ||
638 | addrRx += QStringLiteral("\\[[0-9]{,3}(\\.[0-9]{,3}){3}\\]"); | 637 | addrRx += QStringLiteral("\\[[0-9]{1,3}(\\.[0-9]{1,3}){3}\\]"); | ||
{,3} would be {0,3}, not {1,3} Would this allow to minimize the amount of changes made here? I'm worried about regressions. dfaure: `{,3}` would be `{0,3}`, not `{1,3}`
Would this allow to minimize the amount of changes made… | |||||
From the unit test, this is to match literal domains, [123.123.123.123]; but the old code would match: const QString str = QStringLiteral("[1.1.1.1]"); const QString str2 = QStringLiteral("[.1.1.1]"); const QString str3 = QStringLiteral("[..1.1]"); const QString pattern = QStringLiteral("\\[[0-9]{,3}(\\.[0-9]{,3}){3}\\]"); QRegExp re(pattern); qDebug() << re.exactMatch(str); // true qDebug() << re.exactMatch(str2); // true qDebug() << re.exactMatch(str3); // true So I dare say even in the old code it should have been {1,3}. And I found that:
const QString str = QStringLiteral("a"); const QString oldPattern = QStringLiteral("\\d{,3}"); // QRegExp _is_ old QRegExp oldRe(oldPattern); qDebug() << (oldRe.indexIn(str) != -1); // true, because it's {0,3} const QString pattern = QStringLiteral("\\d{,3}"); QRegularExpression re(pattern); qDebug() << re.match(str).hasMatch(); // false, because it's {1,3} (^ candidate for QRegExp (its docs literally mention {,n}) to QRegularExpression porting notes in QRegularExpression docs, if you can pass that to Giuseppe D'Angelo :)). ahmadsamir: From the unit test, this is to match literal domains, [123.123.123.123]; but the old code would… | |||||
I added two more rows to the literal domain test code and found that, my code is wrong too: addrRx += QStringLiteral("\\[([0-9]{1,3}\\.){1,3}([0-9]{1,3})\\]"); is wrong, it should be: addrRx += QStringLiteral("\\[([0-9]{1,3}\\.){3}([0-9]{1,3})\\]"); ^^^ I'll upload a new diff. ahmadsamir: I added two more rows to the literal domain test code and found that, my code is wrong too… | |||||
dfaure: I'm confused, you still have `1,3` here, rather than `0,3` | |||||
I had replied to your comment but didn't "submit" my comments (stooopid, sorry). ahmadsamir: I had replied to your comment but didn't "submit" my comments (stooopid, sorry).
| |||||
639 | } else { | 638 | } else { | ||
640 | addrRx += QStringLiteral("[\\w-#]+(\\.[\\w-#]+)*"); | 639 | addrRx += QStringLiteral("[\\w#-]+(\\.[\\w#-]+)*"); | ||
641 | } | 640 | } | ||
642 | QRegExp rx(addrRx); | 641 | | ||
643 | return rx.exactMatch(aStr) && !tooManyAtsFlag; | 642 | const QRegularExpression rx(QRegularExpression::anchoredPattern(addrRx) | ||
643 | , QRegularExpression::UseUnicodePropertiesOption); | ||||
dfaure: A bit weird that this comma isn't at the end of the previous line instead :) | |||||
644 | return rx.match(aStr).hasMatch() && !tooManyAtsFlag; | ||||
644 | } | 645 | } | ||
645 | 646 | | |||
646 | //----------------------------------------------------------------------------- | 647 | //----------------------------------------------------------------------------- | ||
647 | QString KEmailAddress::simpleEmailAddressErrorMsg() | 648 | QString KEmailAddress::simpleEmailAddressErrorMsg() | ||
648 | { | 649 | { | ||
649 | return QObject::tr("The email address you entered is not valid because it " | 650 | return QObject::tr("The email address you entered is not valid because it " | ||
650 | "does not seem to contain an actual email address, i.e. " | 651 | "does not seem to contain an actual email address, i.e. " | ||
651 | "something of the form joe@example.org."); | 652 | "something of the form joe@example.org."); | ||
▲ Show 20 Lines • Show All 456 Lines • ▼ Show 20 Line(s) | 1085 | { | |||
1108 | return escaped; | 1109 | return escaped; | ||
1109 | } | 1110 | } | ||
1110 | 1111 | | |||
1111 | //----------------------------------------------------------------------------- | 1112 | //----------------------------------------------------------------------------- | ||
1112 | QString KEmailAddress::quoteNameIfNecessary(const QString &str) | 1113 | QString KEmailAddress::quoteNameIfNecessary(const QString &str) | ||
1113 | { | 1114 | { | ||
1114 | QString quoted = str; | 1115 | QString quoted = str; | ||
1115 | 1116 | | |||
1116 | QRegExp needQuotes(QStringLiteral("[^ 0-9A-Za-z\\x0080-\\xFFFF]")); | 1117 | const QRegularExpression needQuotes(QStringLiteral("[^ 0-9A-Za-z\\x{0080}-\\x{FFFF}]")); | ||
Also from Giuseppe : the \\x0080-\\xFFFF requires changes to \\x{0080}-\\x{FFFF} dfaure: Also from Giuseppe : the `\\x0080-\\xFFFF` requires changes to `\\x{0080}-\\x{FFFF}`
| |||||
Thank him for me, and thank you for asking him to take a look at it :D; from https://perldoc.perl.org/perlre.html:
so, disaster averted. ahmadsamir: Thank him for me, and thank you for asking him to take a look at it :D; from https://perldoc. | |||||
1117 | // avoid double quoting | 1118 | // avoid double quoting | ||
1118 | if ((quoted[0] == QLatin1Char('"')) && (quoted[quoted.length() - 1] == QLatin1Char('"'))) { | 1119 | if ((quoted[0] == QLatin1Char('"')) && (quoted[quoted.length() - 1] == QLatin1Char('"'))) { | ||
1119 | quoted = QLatin1String("\"") + escapeQuotes(quoted.mid(1, quoted.length() - 2)) + QLatin1String("\""); | 1120 | quoted = QLatin1String("\"") + escapeQuotes(quoted.mid(1, quoted.length() - 2)) + QLatin1String("\""); | ||
1120 | } else if (quoted.indexOf(needQuotes) != -1) { | 1121 | } else if (quoted.indexOf(needQuotes) != -1) { | ||
1121 | quoted = QLatin1String("\"") + escapeQuotes(quoted) + QLatin1String("\""); | 1122 | quoted = QLatin1String("\"") + escapeQuotes(quoted) + QLatin1String("\""); | ||
1122 | } | 1123 | } | ||
1123 | 1124 | | |||
1124 | return quoted; | 1125 | return quoted; | ||
Show All 16 Lines |
[pre-existing: this bool is never set to true....]