diff --git a/blink/renderer/modules/manifest/manifest_parser.cc b/blink/renderer/modules/manifest/manifest_parser.cc index 0d7115d5fdbb..0885423c3eb4 100644 --- a/blink/renderer/modules/manifest/manifest_parser.cc +++ b/blink/renderer/modules/manifest/manifest_parser.cc @@ -340,23 +340,27 @@ String ManifestParser::ParseId(const JSONObject* object, return String(); } - absl::optional id = ParseString(object, "id", NoTrim); - if (id.has_value()) { + if (!start_url.IsValid()) { + ManifestUmaUtil::ParseIdResult( + ManifestUmaUtil::ParseIdResultType::kInvalidStartUrl); + return String(); + } + KURL start_url_origin = KURL(SecurityOrigin::Create(start_url)->ToString()); + + KURL id = ParseURL(object, "id", start_url_origin, + ParseURLRestrictions::kSameOriginOnly); + if (id.IsValid()) { ManifestUmaUtil::ParseIdResult( ManifestUmaUtil::ParseIdResultType::kSucceed); - return *id; } else { - // If id is not specified, sets to start_url with origin stripped. - if (start_url.IsValid()) { - ManifestUmaUtil::ParseIdResult( - ManifestUmaUtil::ParseIdResultType::kDefaultToStartUrl); - return start_url.GetString().Substring(start_url.PathStart() + 1); - } else { - ManifestUmaUtil::ParseIdResult( - ManifestUmaUtil::ParseIdResultType::kInvalidStartUrl); - return String(); - } + // If id is not specified, sets to start_url + ManifestUmaUtil::ParseIdResult( + ManifestUmaUtil::ParseIdResultType::kDefaultToStartUrl); + id = start_url; } + // TODO(https://crbug.com/1231765): rename the field to relative_id to reflect + // the actual value. + return id.GetString().Substring(id.PathStart() + 1); } KURL ManifestParser::ParseStartURL(const JSONObject* object) { diff --git a/blink/renderer/modules/manifest/manifest_parser_unittest.cc b/blink/renderer/modules/manifest/manifest_parser_unittest.cc index 3eb707b1b726..5faaaf301bfe 100644 --- a/blink/renderer/modules/manifest/manifest_parser_unittest.cc +++ b/blink/renderer/modules/manifest/manifest_parser_unittest.cc @@ -274,6 +274,13 @@ TEST_F(ManifestParserTest, IdParseRules) { ASSERT_EQ(0u, GetErrorCount()); EXPECT_EQ("start?query=a", manifest->id); } + // Invalid type. + { + auto& manifest = + ParseManifest("{\"start_url\": \"/start?query=a\", \"id\": 1}"); + ASSERT_EQ(1u, GetErrorCount()); + EXPECT_EQ("start?query=a", manifest->id); + } // Empty string. { auto& manifest = @@ -281,6 +288,36 @@ TEST_F(ManifestParserTest, IdParseRules) { ASSERT_EQ(0u, GetErrorCount()); EXPECT_EQ("", manifest->id); } + // Full url. + { + auto& manifest = ParseManifest( + "{ \"start_url\": \"/start?query=a\", \"id\": \"http://foo.com/foo\" " + "}"); + ASSERT_EQ(0u, GetErrorCount()); + EXPECT_EQ("foo", manifest->id); + } + // Full url with different origin. + { + auto& manifest = ParseManifest( + "{ \"start_url\": \"/start?query=a\", \"id\": " + "\"http://another.com/foo\" }"); + ASSERT_EQ(1u, GetErrorCount()); + EXPECT_EQ("start?query=a", manifest->id); + } + // Relative path + { + auto& manifest = + ParseManifest("{ \"start_url\": \"/start?query=a\", \"id\": \".\" }"); + ASSERT_EQ(0u, GetErrorCount()); + EXPECT_EQ("", manifest->id); + } + // Absolute path + { + auto& manifest = + ParseManifest("{ \"start_url\": \"/start?query=a\", \"id\": \"/\" }"); + ASSERT_EQ(0u, GetErrorCount()); + EXPECT_EQ("", manifest->id); + } // Smoke test. { auto& manifest =