Commit fad00e37 authored by Eugene But's avatar Eugene But Committed by Commit Bot

[ios] Do not store navigation items with extra long URLs

According to Session.WebStates.NavigationItem.SerializedURLSize and
Session.WebStates.NavigationItem.SerializedReferrerURLSize histograms
stored URLs might be very long, which can have negative impact on
stability.

InvalidUrlTabHelper will not allow navigations to URLs longer than
kMaxURLChars, so there is no point in storing navigation items with
such long URLs. InvalidUrlTabHelper depends on UseJSForErrorPage flag
and will be enabled some time this fall.

Bug: 1082279
Change-Id: I2839bc6fbe8a1335c710e6afe28352f0f886698b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324724Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarAli Juma <ajuma@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792737}
parent fd01e9d1
......@@ -21,7 +21,12 @@ CRWNavigationItemStorage* NavigationItemStorageBuilder::BuildStorage(
CRWNavigationItemStorage* storage = [[CRWNavigationItemStorage alloc] init];
storage.virtualURL = navigation_item->GetVirtualURL();
storage.URL = navigation_item->GetURL();
storage.referrer = navigation_item->GetReferrer();
// Use default referrer if URL is longer than allowed. Navigation items with
// these long URLs will not be serialized, so there is no point in keeping
// referrer URL.
if (navigation_item->GetReferrer().url.spec().size() <= url::kMaxURLChars) {
storage.referrer = navigation_item->GetReferrer();
}
storage.timestamp = navigation_item->GetTimestamp();
storage.title = navigation_item->GetTitle();
storage.displayState = navigation_item->GetPageDisplayState();
......
......@@ -49,7 +49,8 @@ CRWSessionStorage* SessionStorageBuilder::BuildStorage(
++index) {
web::NavigationItemImpl* item =
navigation_manager->GetNavigationItemImplAtIndex(index);
if (item->ShouldSkipSerialization()) {
if (item->ShouldSkipSerialization() ||
item->GetURL().spec().size() > url::kMaxURLChars) {
if (index <= originalIndex) {
session_storage.lastCommittedItemIndex--;
}
......
......@@ -4,6 +4,7 @@
#import "ios/web/navigation/session_storage_builder.h"
#include "base/strings/sys_string_conversions.h"
#import "ios/web/navigation/wk_navigation_util.h"
#import "ios/web/public/session/crw_navigation_item_storage.h"
#import "ios/web/public/session/crw_session_storage.h"
......@@ -134,4 +135,39 @@ TEST_F(SessionStorageBuilderTest, ShouldSkipSerializationItems) {
}
}
// Tests building storage for session that has URL longer than
// url::kMaxURLChars.
TEST_F(SessionStorageBuilderTest, SkipLongUrls) {
// Create WebState with navigation item count that exceeds kMaxSessionSize.
WebState::CreateParams params(GetBrowserState());
WebStateWithMockProxy web_state(params);
NSString* long_url =
[@"https://" stringByPaddingToLength:url::kMaxURLChars + 1
withString:@"a"
startingAtIndex:0];
NSString* normal_url = @"https://foo.test";
NSString* const current_url = normal_url;
[web_state.fake_wk_list() setCurrentURL:normal_url
backListURLs:@[ long_url ]
forwardListURLs:nil];
OCMStub([web_state.mock_web_view() URL])
.andReturn([NSURL URLWithString:current_url]);
NavigationManager* navigation_manager = web_state.GetNavigationManager();
ASSERT_EQ(2, navigation_manager->GetItemCount());
web_state.GetNavigationManagerImpl()
.GetNavigationItemImplAtIndex(1)
->SetReferrer(web::Referrer(GURL(base::SysNSStringToUTF8(long_url)),
web::ReferrerPolicy::ReferrerPolicyDefault));
// Verify that storage has single item and that item does not have a referrer.
SessionStorageBuilder builder;
CRWSessionStorage* storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(1U, storage.itemStorages.count);
EXPECT_EQ(GURL::EmptyGURL(), [storage.itemStorages.firstObject referrer].url);
}
} // namespace web
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