summaryrefslogtreecommitdiffstats
path: root/src/corelib/tools
diff options
context:
space:
mode:
authorEdward Welbourne <edward.welbourne@theqtcompany.com>2016-01-18 13:03:49 +0100
committerEdward Welbourne <edward.welbourne@theqtcompany.com>2016-04-06 14:52:16 +0000
commit6a7f5dab0d6e86950f87123c69724018aa140770 (patch)
treed143fd81b287b671d33a550667edac4e68dfa387 /src/corelib/tools
parent6fae048af52622db99221a98fd721b59a1bca260 (diff)
Explain QTimeZonePrivate::isValidId a bit more carefully.
Its "rules" are actually guidelines, its suggested regex was wrong, its actual implementation was fuzzier than its documentation suggested and the exception it tacitly permitted should be distinguished from the stricter rules it otherwise appears to implement. There was also a redundant check ('-' had been handled earlier in the chained if). Explain why the situation is tricky, fix the regex mentioned (making it more readable, too) and note what might be worth doing a little more fussily, without actually changing code behavior. Change-Id: I93fa0da0640a134e5d84011b435a186576824063 Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
Diffstat (limited to 'src/corelib/tools')
-rw-r--r--src/corelib/tools/qtimezoneprivate.cpp57
1 files changed, 40 insertions, 17 deletions
diff --git a/src/corelib/tools/qtimezoneprivate.cpp b/src/corelib/tools/qtimezoneprivate.cpp
index ab8332aea3..be53a07591 100644
--- a/src/corelib/tools/qtimezoneprivate.cpp
+++ b/src/corelib/tools/qtimezoneprivate.cpp
@@ -441,22 +441,45 @@ QTimeZone::OffsetData QTimeZonePrivate::toOffsetData(const QTimeZonePrivate::Dat
return offsetData;
}
-// If the format of the ID is valid
+// Is the format of the ID valid ?
bool QTimeZonePrivate::isValidId(const QByteArray &ianaId)
{
- // Rules for defining TZ/IANA names as per ftp://ftp.iana.org/tz/code/Theory
- // 1. Use only valid POSIX file name components
- // 2. Within a file name component, use only ASCII letters, `.', `-' and `_'.
- // 3. Do not use digits
- // 4. A file name component must not exceed 14 characters or start with `-'
- // Aliases such as "Etc/GMT+7" and "SystemV/EST5EDT" are valid so we need to accept digits, ':', and '+'.
-
- // The following would be preferable if QRegExp would work on QByteArrays directly:
- // const QRegExp rx(QStringLiteral("[a-z0-9:+._][a-z0-9:+._-]{,13}(?:/[a-z0-9:+._][a-z0-9:+._-]{,13})*"),
- // Qt::CaseInsensitive);
- // return rx.exactMatch(ianaId);
-
- // hand-rolled version:
+ /*
+ Main rules for defining TZ/IANA names as per ftp://ftp.iana.org/tz/code/Theory
+ 1. Use only valid POSIX file name components
+ 2. Within a file name component, use only ASCII letters, `.', `-' and `_'.
+ 3. Do not use digits (except in a [+-]\d+ suffix, when used).
+ 4. A file name component must not exceed 14 characters or start with `-'
+ However, the rules are really guidelines - a later one says
+ - Do not change established names if they only marginally violate the
+ above rules.
+ We may, therefore, need to be a bit slack in our check here, if we hit
+ legitimate exceptions in real time-zone databases.
+
+ In particular, aliases such as "Etc/GMT+7" and "SystemV/EST5EDT" are valid
+ so we need to accept digits, ':', and '+'; aliases typically have the form
+ of POSIX TZ strings, which allow a suffix to a proper IANA name. A POSIX
+ suffix starts with an offset (as in GMT+7) and may continue with another
+ name (as in EST5EDT, giving the DST name of the zone); a further offset is
+ allowed (for DST). The ("hard to describe and [...] error-prone in
+ practice") POSIX form even allows a suffix giving the dates (and
+ optionally times) of the annual DST transitions. Hopefully, no TZ aliases
+ go that far, but we at least need to accept an offset and (single
+ fragment) DST-name.
+
+ But for the legacy complications, the following would be preferable if
+ QRegExp would work on QByteArrays directly:
+ const QRegExp rx(QStringLiteral("[a-z+._][a-z+._-]{,13}"
+ "(?:/[a-z+._][a-z+._-]{,13})*"
+ // Optional suffix:
+ "(?:[+-]?\d{1,2}(?::\d{1,2}){,2}" // offset
+ // one name fragment (DST):
+ "(?:[a-z+._][a-z+._-]{,13})?)"),
+ Qt::CaseInsensitive);
+ return rx.exactMatch(ianaId);
+ */
+
+ // Somewhat slack hand-rolled version:
const int MinSectionLength = 1;
const int MaxSectionLength = 14;
int sectionLength = 0;
@@ -472,11 +495,11 @@ bool QTimeZonePrivate::isValidId(const QByteArray &ianaId)
} else if (!(ch >= 'a' && ch <= 'z')
&& !(ch >= 'A' && ch <= 'Z')
&& !(ch == '_')
+ && !(ch == '.')
+ // Should ideally check these only happen as an offset:
&& !(ch >= '0' && ch <= '9')
- && !(ch == '-')
&& !(ch == '+')
- && !(ch == ':')
- && !(ch == '.')) {
+ && !(ch == ':')) {
return false; // violates (2)
}
}