Commit 0c2d3753 authored by Majid Valipour's avatar Majid Valipour Committed by Commit Bot

[WebOTP] Improve sms parsing

Changes:
- Fix regex to accept very short otp code: #(.[]+) -> #([]+)
- Add tests to ensure we reject forbidden host characters
- Add additional tests around number of spaces expected

Followup patch will allow host to accept unicode.

Change-Id: I7cbeabe31edd9c9ead7ae78425c8760f9c78ce9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2248071
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: default avatarSam Goto <goto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780306}
parent 49a3401d
......@@ -16,8 +16,8 @@
namespace content {
// SMS one-time-passcode format:
// https://github.com/WebKit/explainers/blob/master/sms-one-time-code-format/README.md
constexpr char kOtpFormatRegex[] = "(?:^|\\s)@([a-zA-Z0-9.-]+) #(.[^#\\s]+)";
// https://wicg.github.io/sms-one-time-codes/#parsing
constexpr char kOtpFormatRegex[] = "(?:^|\\s)@([a-zA-Z0-9.-]+) #([^#\\s]+)";
SmsParser::Result::Result(const url::Origin& origin,
const std::string& one_time_code)
......
......@@ -7,6 +7,7 @@
#include <string>
#include "base/optional.h"
#include "base/strings/stringprintf.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
#include "url/origin.h"
......@@ -43,6 +44,18 @@ TEST(SmsParserTest, NoSpace) {
ASSERT_FALSE(SmsParser::Parse("@example.com#12345"));
}
TEST(SmsParserTest, MultipleSpace) {
ASSERT_FALSE(SmsParser::Parse("@example.com #12345"));
}
TEST(SmsParserTest, WhiteSpaceThatIsNotSpace) {
ASSERT_FALSE(SmsParser::Parse("@example.com\t#12345"));
}
TEST(SmsParserTest, WordInBetween) {
ASSERT_FALSE(SmsParser::Parse("@example.com random #12345"));
}
TEST(SmsParserTest, InvalidUrl) {
ASSERT_FALSE(SmsParser::Parse("@//example.com #123"));
}
......@@ -102,6 +115,46 @@ TEST(SmsParserTest, Dashes) {
ParseOrigin("@web-otp-example.com #123"));
}
TEST(SmsParserTest, CapitalLetters) {
EXPECT_EQ(url::Origin::Create(GURL("https://can-contain-CAPITAL.com")),
ParseOrigin("@can-contain-CAPITAL.com #123"));
}
TEST(SmsParserTest, Numbers) {
EXPECT_EQ(url::Origin::Create(GURL("https://can-contain-number-9870.com")),
ParseOrigin("@can-contain-number-9870.com #123"));
}
TEST(SmsParserTest, ForbiddenCharacters) {
// TODO(majidvp): Domains with unicode characters are valid.
// See: https://url.spec.whatwg.org/#concept-domain-to-ascii
// EXPECT_EQ(url::Origin::Create(GURL("can-contain-unicode-like-חומוס.com")),
// ParseOrigin("@can-contain-unicode-like-חומוס.com #123"));
// Forbidden codepoints https://url.spec.whatwg.org/#forbidden-host-code-point
const char forbidden_chars[] = {'\x00' /* null */,
'\x09' /* TAB */,
'\x0A' /* LF */,
'\x0D' /* CR */,
' ',
'#',
'%',
'/',
':',
'<',
'>',
'?',
'@',
'[',
'\\',
']',
'^'};
for (char c : forbidden_chars) {
ASSERT_FALSE(
SmsParser::Parse(base::StringPrintf("@cannot-contain-%c #123456", c)));
}
}
TEST(SmsParserTest, Newlines) {
EXPECT_EQ(url::Origin::Create(GURL("https://example.com")),
ParseOrigin("hello world\n@example.com #123\n"));
......@@ -149,7 +202,9 @@ TEST(SmsParserTest, OneTimeCodeCharRanges) {
ParseOTP("@example.com #human-readable-words-like-sillyface"));
EXPECT_EQ("can-it-be-super-lengthy-like-a-lot",
ParseOTP("@example.com #can-it-be-super-lengthy-like-a-lot"));
EXPECT_EQ("1", ParseOTP("@example.com #1 can be short"));
EXPECT_EQ("otp", ParseOTP("@example.com #otp with space"));
EXPECT_EQ("otp", ParseOTP("@example.com #otp\twith with tab"));
}
} // namespace content
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment